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
UI: Reflect search in URL #16527
base: next
Are you sure you want to change the base?
UI: Reflect search in URL #16527
Conversation
☁️ Nx Cloud ReportCI is running/has finished running commands for commit a62b0ff. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this branch ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
f040895
to
3b19b49
Compare
Can we add a |
3b19b49
to
29f5903
Compare
Thanks for the PR @jh3y! I tagged in @ghengeveld who built search and knows about the URL structure. |
Rad! Thanks @domyen 🙏 |
29f5903
to
d2b4929
Compare
d2b4929
to
026d267
Compare
Hey @ghengeveld could you please take a look at this whenever you've got time? |
Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks! |
Bumping this so Happy New Year! 🎉 |
Hey @jh3y thanks a lot for your patience and happy new year! I'll make sure this PR is reviewed this week |
I'll take another look at this. Thanks for the links @ghengeveld 🙏 Will take me a minute to refresh my context knowledge for this again 😄 |
Great, thanks. It's appreciated! |
171f659
to
964f341
Compare
Updated @ghengeveld 🚀
Only thing is that the core tests are timing out. Not sure if it's a case of giving them a rerun as it's related to something with Vue 🤔 Thanks! ʕ´•ᴥ•`ʔ |
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 58272ef. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
i caught this up from not realising i was still mid-merge, i fixed those and reworked the tests (so it shows up as a merge commit but we'll squash it anyway i imagine when it lands). TLDR of what I changed:
|
Legend 🙏 A lot has changed since first opening this PR. Looking back on this, I got it rebased at whatever stage and then couldn't seem to get the various tests passing at that time. Thanks for taking a look! |
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.
Impressive work bringing this up to date!!
code/ui/manager/src/components/sidebar/__tests__/Search.test.tsx
Outdated
Show resolved
Hide resolved
Amazing work!! The story failures in Chromatic are not related to this PR. I just made another PR that fixes them so once that's merged, let's just get the commits in here to fix that. |
the current build failure seems to be mostly due to having stories with args. im slightly baffled by this because the both export const Simple: Story = {}; but the search stories fail type checking.. edit: think i got it! 🙏 edit2: yup, it builds 🎉 |
@43081j that other PR was merged so I updated this branch (and fixed a small merge conflict), it should all be good now! |
Issue: #16287
Integrate way to reflect search("filter") in URL bar in order to share the URL for hosted Storybook instances.
What I did
Sidebar
Search
initialQuery
fromURLSearchParams
and use thefilter
value.Downshift
render to update the URL bar withwindow.history.replaceState
.How to test
If your answer is yes to any of these, please make sure to include it in your PR.
Tests are under
Search.test.js
. Tests populating the input field in addition to updatewindow.location
via mockedwindow.history
👍