-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat(cxl-ui): add cxl-search-filters
and related components
#297
base: feat/dashboard
Are you sure you want to change the base?
feat(cxl-ui): add cxl-search-filters
and related components
#297
Conversation
6c36711
to
e5fe8f6
Compare
d1b7a38
to
be528c3
Compare
A few notes:
|
For easier review but also tracking code it'd be better to have every component as separate commit. |
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.
import cxlDashboardFilterHeaderStyles from '../styles/cxl-filter-header-css.js'; | ||
|
||
const supportsScrollEndEvent = 'onscrollend' in window | ||
const isFirefox = document.scrollingElement.scrollLeftMax !== undefined |
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.
What's with Firefox that we need special flag?
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.
When in smaller screens, I implemented scrolling for the tabs, like in the design. I also implemented scroll snapping, so that it aligns to the start of the closest tab when the user stops scrolling. In Firefox, the smooth scrolling behavior causes a bug that prevents it from scrolling further on very specific screen sizes and tab widths, so I'm just using that to disable smooth scrolling in that case
render () { | ||
return html` | ||
<div class="container"> | ||
<div class="tabs"> |
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.
What do you think about using https://vaadin.com/docs/latest/components/tabs here @freudFlintstone? Beneficial or overkill?
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.
I tried, really hard... but I wasted a day on them, it was not looking as good as the design, and in the end it would actually not work in this specific case, because of how FacetWP works. Also, overkill, since we only need the looks and basically none of the functionality.
3fdb5e5
to
413afce
Compare
Do you want me to go back and make separate commits? It's doable |
Yes, please |
413afce
to
ea23944
Compare
ea23944
to
bb559b1
Compare
4ebb721
to
47c5739
Compare
3e83479
to
7c4b890
Compare
Task: https://app.clickup.com/t/861mzgnx5
These changes depend on being able to replace some of the elements rendered by the FacetWP plugin. Here's a gist for a similar problem:
https://gist.github.com/MWDelaney/7f3ff797041bac5ef9918112d6a43816