-
Notifications
You must be signed in to change notification settings - Fork 12
added core features examples #18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
markawm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Seeing it now you've done it, I think I prefer the single app approach too!
One other thing we talked about was evaluating with a context, would be good to add this. And to include a short description in the comments contrasting the two approaches:
- providing the custom property value via a
setCustomXXXProperty()(static or dynamic). - providing the value inline in the flag evaluation (e.g.
myFlag.isEnabled({userEmail: 'me@example.org'}).
| Rox.setCustomNumberProperty('accountAgeInDays', () => { | ||
| return 45; // User registered 45 days ago | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also talked about showing an example of a dynamic property. This might be a good property to show that with, being temporal.
e.g. `if account.signupDate is more than 45 days before current date".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(also cover dynamic properties in the comments above).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated. let me know if I still misunderstood something.
| */ | ||
|
|
||
| // String property example - User tier for subscription-based targeting | ||
| Rox.setCustomStringProperty('userTier', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the setCustomXXXProperty() function lets you pass either a a value or a calculator function (need to check), and for these simple/fixed ones we might as well show the simple case, i.e. setCustomStringProperty('userTier', 'premium').
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
| Rox.setCustomDateProperty('registrationDate', () => { | ||
| // In production, get this from user's actual registration date | ||
| // Example: return new Date(user.registeredAt); | ||
| return new Date('2024-01-15'); // User registered on Jan 15, 2024 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we could easily introduce something like a user object? Then this could feel a bit more real because it would be set to user.signUpDate or something. (Obviously the focus is the SDK API, so don't go building a user management/login type system. Just hard-code it somewhere with a comment saying "this would have come from the login process..." or something).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes.. I have added this
const user = {
id: 'user-123',
email: 'demo@example.com',
tier: 'premium', // subscription tier: 'free', 'premium', 'enterprise'
signUpDate: new Date('2024-01-15'),
isBetaTester: true,
};
| * | ||
| * Note: Dynamic API flags do NOT need to be registered | ||
| */ | ||
| Rox.register('', flags) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wdyt about showing off namespace functionality? We missed talking about it yesterday. e.g. create two separate flags containers for different types/groups of flags, and register them under different namespaces. I'm struggling to think of a good example atm, but something like:
uxFlags = {
fontColour
fontSize
...
}
featureFlags = {
showMessage
message
}
Rox.register('', featureFlags)
Rox.register('ux', uxFlags)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good idea. updated.
src/App.tsx
Outdated
| const showDynamicMessage = Rox.dynamicApi.isEnabled('showDynamicMessage', false); | ||
|
|
||
| // String flag using Dynamic API - The dynamic message text to display | ||
| // Default: 'This is from dynamic API flags. Try changing some flag values!' | ||
| const dynamicMessage = Rox.dynamicApi.value('dynamicMessage', 'This is from dynamic API flags. Try changing some flag values!'); | ||
|
|
||
| // String flag using Dynamic API - Font color for the dynamic message | ||
| // Default: 'Green' | ||
| const dynamicFontColor = Rox.dynamicApi.value('dynamicFontColor', 'Green'); | ||
|
|
||
| // Number flag using Dynamic API - Font size in pixels for the dynamic message | ||
| // Default: 16 | ||
| const dynamicFontSize = Rox.dynamicApi.getNumber('dynamicFontSize', 16); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we need to introduce a whole new set of flags? We could just show that its an alternative way to access the same flags and carry on using the standard fontSize, fontColour, etc.
(but you could still display them twice if you like, so that you get a different JS variable to use).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay. I removed dynamic api set of flags and mixed with static api . and make a comment that customer can user either way to all values.
package.json
Outdated
| "react": "^18.2.0", | ||
| "react-dom": "^18.2.0", | ||
| "rox-browser": "^5.4.12" | ||
| "rox-browser": "6.0.8" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might as well allow upgrades here as we keep releasing new fixes. In fact, since we always try to keep backwards compatibility, you could probably pin just the first digit. (& even if we did break it and create v7, we would want to be updating this at the same time).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep.. this make sense.. I have updated.
markawm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good! Just fixed a small typo.
Co-authored-by: Mark Wynn-Mackenzie <mwynnmackenzie@cloudbees.com>
in this PRI have added the React example application to demonstrate advanced CloudBees Feature Management capabilities including Dynamic API, Impression Handler, Flag Freeze and Custom Properties and setuo verbose debug level.
CBP-4117