-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Test URL variables in design #15534
Test URL variables in design #15534
Conversation
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
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.
Nice work Pete. I played around with this locally and couldn't find any issues with it functionally. It handled everything I threw at it, so nice job 👌
Just a few style points:
- We shouldn't show this at all if your screen doesn't use any URL params. Right now it always shows, but serves no purpose if you aren't using any params
- Is it worth updating the copy slightly to make it clear it's temporary? With the new state panel we are very explicit with "Set temporary value for design preview". This is basically an identical feature for URL params. I'm not totally set on this though - I don't really mind either way.
- Style wise it's a bit unique, but that's probably fine. It might be nice to at least align it with our other settings though, which would mean making hardcoded prefix section 98px wide rather than 40%. Could we maybe update the label to be the same colour as the rest of the settings labels too? And probably also update the casing to remove the capital letters other than the first one.
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.
LGTM!
Description
This PR adds the ability for users to test their URL variables. When building screens with dynamic URL parameters (like
/employees/:id
), this lets the user provide an example value so they can see how it affects the screenAlso, to make it easier to understand what is needed, the placeholder is based on the route of the screen, so say for
/employees/:test/validate/:id
, the placeholder field shows an example like "1/validate/2".Screenshots
Screen.Recording.2025-02-11.at.13.34.38.mov
Launchcontrol
Feature branch env
Feature Branch Link