Skip to content
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.

Sign up for GitHub

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

Jump to bottom

chore(styles): apply header list styles to buttons #4785

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

chore(styles): apply header list styles to buttons #4785

wants to merge 11 commits into from
+19 −0

Conversation

myrta2302
Copy link
Contributor

No description provided.

@myrta2302 myrta2302 requested a review from a team as a code owner February 25, 2025 15:54
@myrta2302 myrta2302 requested a review from leagrdv February 25, 2025 15:54
@myrta2302 myrta2302 linked an issue Feb 25, 2025 that may be closed by this pull request
Correctly style button in the local header #4781
Open
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Feb 25, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: f75f848

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 15 packages
Name Type
@swisspost/design-system-styles Patch
@swisspost/design-system-components-angular-workspace Patch
@swisspost/design-system-components Patch
@swisspost/design-system-documentation Patch
@swisspost/internet-header Patch
@swisspost/design-system-intranet-header-workspace Patch
@swisspost/design-system-nextjs-integration Patch
@swisspost/design-system-styles-primeng-workspace Patch
@swisspost/design-system-intranet-header Patch
@swisspost/design-system-styles-primeng Patch
@swisspost/design-system-components-react Patch
@swisspost/design-system-components-angular Patch
@swisspost/design-system-intranet-header-showcase Patch
@swisspost/design-system-tokens Patch
@swisspost/design-system-icons Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@myrta2302 myrta2302 requested review from alizedebray and removed request for leagrdv February 25, 2025 15:56
@swisspost-bot
Copy link
Contributor

swisspost-bot commented Feb 25, 2025 •
edited
Loading

Related Previews

@leagrdv leagrdv added the Header Everything related to our composible header components label Feb 25, 2025
@myrta2302 myrta2302 requested a review from leagrdv February 25, 2025 16:15
@myrta2302 myrta2302 requested a review from leagrdv February 25, 2025 17:04
alizedebray
alizedebray requested changes Feb 26, 2025
View reviewed changes
packages/styles/src/components/header/index.scss Outdated
a,
> ul.list-inline li button,
Copy link
Contributor

@alizedebray alizedebray Feb 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this selector so specific, is there side effects if you simply use button as we do for the a and post-togglebutton?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was targeting also the language switch buttons.

Copy link
Contributor

@alizedebray alizedebray Feb 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure? The selector bellow is &:not(post-mainnavigation *, post-language-switch *) so it should exclude the language switch buttons.

@leagrdv
Copy link
Contributor

leagrdv commented Feb 26, 2025

Approved on my side

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

alizedebray
alizedebray reviewed Feb 27, 2025
View reviewed changes
Copy link
Contributor

@alizedebray alizedebray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update the PR with the main branch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alizedebray alizedebray alizedebray requested changes

@leagrdv leagrdv Awaiting requested review from leagrdv

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
Header Everything related to our composible header components
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Correctly style button in the local header
4 participants
@myrta2302 @swisspost-bot @leagrdv @alizedebray