-
Notifications
You must be signed in to change notification settings - Fork 147
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
Add URL string union hook to better integrate url params with Tab components #5111
Conversation
Skipping CI for Draft Pull Request. |
Current dependencies on/for this PR: This comment was auto-generated by Graphite. |
Images are ready for the commit at caa4d47. To use with deploy scripts, first |
735b6a0
to
56dd6fc
Compare
81b074b
to
16bf636
Compare
56dd6fc
to
f9fd6b1
Compare
f9fd6b1
to
bdaf3ad
Compare
expect(initialValidResult.current[0]).toBe('Beta'); | ||
expect(params.get('urlKey')).toBe('Beta'); | ||
|
||
const { result: initialInvalidResult } = renderHook( |
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.
Is this the start of a separate test?
- See comment above: if the URL param already contains a valid value
- See comment below: when the URL param is invalid
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.
It can be: 'should use the default value when an invalid value is entered directly into the URL'
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.
Wow, you keep teaching me more TypeScript!
One comment about unit tests.
bdaf3ad
to
caa4d47
Compare
Description
This adds a new hook that simplifies using a string union safely with a URL parameter. This should handle any case where we have a finite list of strings that are valid as a URL parameter, such as the current use case of having a known set of PF
Tab
components that we want to integrate with the URL.Even though this is for VM 2.0 right now, I'll add the @stackrox/ui team for awareness in case this is useful elsewhere.
This was actually a net removal of code, before comments and tests...
Checklist
If any of these don't apply, please comment below.
Testing Performed
Unit tests.
Cycle through tab clicks and ensure the correct tab is highlighted and that the URL parameter changes accordingly.
Visit the page via direct navigation with a non-default tab parameter selected and verify that the correct tab is selected.
Visit the page via direct navigation with an invalid parameter value and verify that the default tab is selected and parameter is changed to the default.