Skip to content

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

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

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

Conversation

@mcoker

@mcoker mcoker commented Aug 16, 2024

Copy link
Copy Markdown
Contributor

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

patternfly-build commented Aug 16, 2024

Copy link
Copy Markdown
Collaborator

@srambach srambach left a comment

Copy link
Copy Markdown
Member

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?

@srambach srambach left a comment

Copy link
Copy Markdown
Member

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

mcoker commented Aug 20, 2024

Copy link
Copy Markdown
Contributor Author

@srambach thanks for the meeting notes!

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

Copy link
Copy Markdown
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