-
Notifications
You must be signed in to change notification settings - Fork 14
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(component): improve header overflow handling #4273
base: main
Are you sure you want to change the base?
Conversation
Related Previews |
|
This reverts commit 8f8cdb6.
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.
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.
There is no changeset file.
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.
As mentioned before, we could replace the implemented breakpoint detection by just using the now existing breakpoints
utility in the components
package.
You can see how it works in the <post-footer>
component or come to me and ask, if you have questions.
I don't say it's mandatory to implement it here, but it would probably make a lot of sense.
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.
Can you create a ticket for that?
.left-scroll-button { | ||
inset-inline-start: 0; | ||
padding-inline-end: 20px; | ||
} | ||
|
||
.right-scroll-button { | ||
inset-inline-end: 0; | ||
padding-inline-start: 20px; | ||
} |
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.
background: var(--post-core-color-brand-white); | ||
padding: var(--post-core-dimension-16); | ||
box-shadow: var(--post-core-elevation-5); | ||
line-height: var(--post-core-line-height-100); |
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 once more recommend: Never use core- or semantic-tokens, but instead use hardcoded values for now.
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.
First time with me!
Can we discuss this? Currently the whole header is using semantic tokens and I would like to understand why you prefer raw values. Also, I would rather handle this in a separate ticket as it would involve a lot of changes not related to this PR.
packages/components/src/components/post-mainnavigation/post-mainnavigation.scss
Show resolved
Hide resolved
packages/components/src/components/post-mainnavigation/post-mainnavigation.tsx
Outdated
Show resolved
Hide resolved
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.
Quality Gate passedIssues Measures |
No description provided.