fix #5957 feat(nimbus-ui): sortable directory columns #6084
Conversation
|
HOLY COW @lmorchard IT'S SO COOL 👏 👏 👏 👏 🎉 🎉 🎉 🎉 🎉 🎉 |
lmorchard
force-pushed
the
5957-directory-table-sort
branch
2 times, most recently
from
6395844
to
5d25299
Compare
lmorchard
changed the title
|
Hmm, almost passing CI. I see an integration_nimbus failure, but not sure if it's intermittent (seems not entirely related to changes) Failed on a re-run, will keep banging on it tomorrow. Maybe the new query params on the home page are tripping up |
|
This is super cool! 👍 Also noticed the minor edge case of adding that sorting triangle can cause some columns to shift. |
| # Load home page and wait for experiment to show in the Drafts tab | ||
| selenium.get(base_url) | ||
| drafts_tab_url = f"{base_url}?tab=drafts" | ||
| selenium.get(drafts_tab_url) | ||
| experiment_found = False | ||
| for attempt in range(45): | ||
| try: | ||
| home = HomePage(selenium, base_url).wait_for_page_to_load() | ||
| home.tabs[-1].click() | ||
| home = HomePage(selenium, drafts_tab_url) |
There was a problem hiding this comment.
This gets the failing integration test to pass, but it's kind of a weird fix.
The thing that it was tripping on was HomePage(selenium, base_url).wait_for_page_to_load(). That just looks for the presence of .directory-table in the DOM.
As far as I can tell, that element is in the DOM when it fails. And this same method is used in other tests and works. But, for some reason, it consistently fails in this test. Can't figure out why.
lmorchard
force-pushed
the
5957-directory-table-sort
branch
from
e53c78d
to
b101d8d
Compare
Because: * We want to make the directory view table sortable by each column * Currently applied sorting should be reflected in the url query parameter. This commit: * Adds support for directory tab selection via query parameter * Adds experiment sorting to DirectoryTable based on location query parameters * Adds SortableColumnTitle component to DirectoryTable to cycle through column sorting modes and update location query parameters * Updates column definitions in DirectoryTable components to support sorting criteria * Adds sorting utilities to lib/experiment.ts * Tweaks mock data to add more sortable variety * Updates tests and stories with router context * Adds useSearchParamsState hook to read and update location query parameters
lmorchard
force-pushed
the
5957-directory-table-sort
branch
from
b101d8d
to
1abcb6e
Compare
|
Alright, I think I've puttered with this enough. CI is passing, going to mark ready for review and spread requests around. Fresh stories:
|
| <p className="p-3"> | ||
| Location:{" "} | ||
| <code data-testid="location"> | ||
| {location.pathname} |
There was a problem hiding this comment.
Looks like this is always rendering /slug/edit
There was a problem hiding this comment.
Yeah, that's just the canned value from mocks. Threw it in there just to make sure the path didn't get altered by the query parameter manipulation
|
Gotta take the r+'s and merge to clear the way for issue #5958 |
Because:
We want to make the directory view table sortable by each column
Currently applied sorting should be reflected in the url query
parameter.
This commit:
Adds support for directory tab selection via query parameter
Adds experiment sorting to DirectoryTable based on location query
parameters
Adds SortableColumnTitle component to DirectoryTable to cycle through
column sorting modes and update location query parameters
Updates column definitions in DirectoryTable components to support
sorting criteria
Adds sorting utilities to lib/experiment.ts
Tweaks mock data to add more sortable variety
Updates tests and stories with router context
Adds useSearchParamsState hook to read and update location query
parameters