8000 fix(toolbar): fix expanded content position in masthead by mcoker · Pull Request #6986 · patternfly/patternfly · GitHub
[go: up one dir, main page]

Skip to content

fix(toolbar): fix expanded content position in masthead#6986

Merged
mcoker merged 1 commit intopatternfly:v6from
mcoker:issue-6918
Aug 20, 2024
Merged

fix(toolbar): fix expanded content position in masthead#6986
mcoker merged 1 commit intopatternfly:v6from
mcoker:issue-6918

Conversation

@mcoker
Copy link
Contributor
@mcoker mcoker commented Aug 16, 2024

fixes #6918

BackstopJS Report.pdf with filter="toolbar|masthead"

The 3 failures are the last 3 in the report, and seem to match the layout I see for these examples v5

@patternfly-build
Copy link
Collaborator
patternfly-build commented Aug 16, 2024

Copy link
Member
@srambach srambach left a comment

Choose a reason for hiding this comment

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

Just one question - the padding is getting reset to 0 above 62rem here - is this by mistake? The box-shadow is never actually removed so was it trying to look more inline but something is still off?

Copy link
Member
@srambach srambach left a comment

Choose a reason for hiding this comment

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

After discussion - all good!
The example in question is a mobile expandable toolbar, and at desktop width the expanded content does not have padding around the toolbar. Currently there really isn't a core example showing a solution for this at desktop width, but there's really not a good use case for this. If it comes up, we can further consider how to handle the padding around expanded content at desktop width.

@mcoker
Copy link
Contributor Author
mcoker commented Aug 20, 2024

@srambach thanks for the meeting notes!

@mcoker mcoker merged commit 96b54fd into patternfly:v6 Aug 20, 2024
@patternfly-build
Copy link
Collaborator

🎉 This PR is included in version 6.0.0-alpha.220 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

0