quilt
quilt copied to clipboard
Scroll area for filters and search results
- [x] Changelog entry (skip if change is not significant to end users, e.g. docs only)
Codecov Report
Attention: 55 lines
in your changes are missing coverage. Please review.
Comparison is base (
b4a9498
) 35.58% compared to head (2ea2d07
) 35.55%.
Additional details and impacted files
@@ Coverage Diff @@
## master #3808 +/- ##
==========================================
- Coverage 35.58% 35.55% -0.04%
==========================================
Files 709 709
Lines 31110 31142 +32
Branches 4651 4667 +16
==========================================
Hits 11071 11071
- Misses 18892 18924 +32
Partials 1147 1147
Flag | Coverage Δ | |
---|---|---|
api-python | 91.22% <ø> (ø) |
|
catalog | 10.57% <0.00%> (-0.02%) |
:arrow_down: |
lambda | 86.26% <ø> (ø) |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
the ux feels too complicated and clumsy. i believe we can make it simpler by doing away with the custom top/bottom indicators and scrollable containers
What do you mean by scrollable containers? Containers for keywords filter? It's no longer scrollable, it has "Show more" button, and show the whole list without scroll
Top/bottom indicators are additional buttons, just to show that something is hidden at top or at bottom. You can use scroll instead of buttons. The similar UX is in vertical tabs component https://v4.mui.com/components/tabs/#vertical-tabs
Containers for keywords filter?
nope
It's no longer scrollable, it has "Show more" button, and show the whole list without scroll
that's good
Top/bottom indicators are additional buttons, just to show that something is hidden at top or at bottom. You can use scroll instead of buttons.
i don't think they are necessary and i believe they just clutter the UI.
i meant smth like this, without too much custom logic and controls, just some css positioning stuff:
Rewrote layout using sticky app header
also, while you're at it, it might make sense to fix the enum options wrapping to multiple lines (simple white-space: nowrap would help)
After reviewing:
- these PR comments
- the test stack on SearchMinimal
- Kevin's comments in Slack
- Our Horizon deliverables and strategic needs
I have reluctantly concluded that @fiskus should not spend any more time on this PR, so he can start working on MOAB deliverables with Dima.
My rationale is that successfully shipping MOAB in January is more important than a highly-polished faceted search this month. @kevinemoore has already said that the observed cosmetic issues, though serious, are not blockers. The reality is that most customers will only install a December release in their 'staging' stacks, and may well provide feedback that obsolete portions of this PR.
@nl0 I believe your options are:
- Ignore this PR and ship the release without it
- Approve and merge this PR even though it does not address your concerns
- Find some other way to address them (e.g., fix them yourself)
- Throw an Exception, if you cannot in good conscience sign off on a release with this feature set
Please let me know at Monday's D&D which way you are leaning. Thanks.