Skip to content

[72657] Improve responsiveness of Banner component#423

Merged
HDinger merged 2 commits intomainfrom
feature/72657-move-banners-to-bottom-right-of-the-screen-so-it-does-not-block-important-content
Mar 11, 2026
Merged

[72657] Improve responsiveness of Banner component#423
HDinger merged 2 commits intomainfrom
feature/72657-move-banners-to-bottom-right-of-the-screen-so-it-does-not-block-important-content

Conversation

@HDinger
Copy link
Collaborator

@HDinger HDinger commented Mar 9, 2026 •
edited
Loading

What are you trying to accomplish?

Let Banner react on container size instead of screen size

Integration

none

List the issues that this change affects.

Closes https://community.openproject.org/wp/72657

@changeset-bot
Copy link

changeset-bot bot commented Mar 9, 2026 •
edited
Loading

🦋 Changeset detected

Latest commit: eca9d03

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

This PR includes changesets to release 1 package
Name Type
@openproject/primer-view-components 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

@HDinger HDinger requested a review from myabc March 9, 2026 15:47
@HDinger HDinger requested a review from bsatarnejad March 9, 2026 15:47
@github-actions
Copy link

github-actions bot commented Mar 9, 2026 •
edited
Loading

⚠️ Visual or ARIA snapshot differences found

Our visual and ARIA snapshot tests found UI differences. Please review the differences by viewing the files changed tab to ensure that the changes were intentional.

Review differences

@myabc myabc changed the title [72657] Improve responsibility of Banner component [72657] Improve responsiveness of Banner component Mar 9, 2026
myabc
myabc reviewed Mar 9, 2026
View reviewed changes
Copy link
Collaborator

@myabc myabc left a comment

Choose a reason for hiding this comment

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

I haven't had a chance to test this yet, but code-wise this looks fine. I like the idea 💯

myabc
myabc reviewed Mar 9, 2026
View reviewed changes
app/components/primer/alpha/banner.pcss

/* `sm` breakpoint variation */
@media (max-width: 543.98px) {
@container (max-width: 543.98px) {
Copy link
Collaborator

@myabc myabc Mar 9, 2026

Choose a reason for hiding this comment

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

Why a sub-pixel amount btw?

Copy link
Collaborator Author

@HDinger HDinger Mar 10, 2026

Choose a reason for hiding this comment

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

The Primer CSS relies everywhere on pixel based breakpoints instead of variables.. I assume that they don't have access to those here, because they originate from another repo. 🤷

Copy link
Collaborator

@myabc myabc Mar 10, 2026

Choose a reason for hiding this comment

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

yeah, but this is subpixel..

myabc
myabc reviewed Mar 9, 2026
View reviewed changes
@HDinger
Copy link
Collaborator Author

HDinger commented Mar 10, 2026

Bildschirmfoto 2026-03-10 um 07 59 52

Thanks for that 🙈 😂

@HDinger HDinger force-pushed the feature/72657-move-banners-to-bottom-right-of-the-screen-so-it-does-not-block-important-content branch from 8a75e23 to 576b200 Compare March 11, 2026 10:17
@HDinger HDinger merged commit 619b6e4 into main Mar 11, 2026
@openproject-primer-view-components openproject-primer-view-components bot mentioned this pull request Mar 11, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@myabc myabc myabc left review comments

@bsatarnejad bsatarnejad bsatarnejad approved these changes

Assignees

No one assigned

Labels

patch release

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@HDinger @myabc @bsatarnejad