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.

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

Fix sidebar header ring overflow-x #9852

Merged
merged 4 commits into from
Nov 26, 2023
Merged

Conversation

4m1n0s
Copy link
Contributor

@4m1n0s 4m1n0s commented Nov 25, 2023

  • Changes have been thoroughly tested to not break existing functionality.
  • New functionality has been documented or existing documentation has been updated to reflect changes.
  • Visual changes are explained in the PR description using a screenshot/recording of before and after.

Before:
image

After:

image

@danharrin danharrin added the bug Something isn't working label Nov 25, 2023
@danharrin danharrin added this to the v3 milestone Nov 25, 2023
Copy link
Member

@zepfietje zepfietje left a comment

Choose a reason for hiding this comment

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

Thanks for reporting this, @4m1n0s.
The use of box-shadow/ring is by design though, to match the positioning of content under the topbar, as the ring doesn't take up any space while the border does.

Could you rework this PR to just prevent overflow-x on the sidebar header? Similar approach is used on the topbar, though it may require a wrapping element.

@zepfietje zepfietje changed the title Fix sidebar header Fix sidebar header ring overflow-x Nov 25, 2023
@zepfietje zepfietje added the ui label Nov 25, 2023
@4m1n0s
Copy link
Contributor Author

4m1n0s commented Nov 25, 2023

@zepfietje in this case i think only a wrapping div like that is enough?

<div class="overflow-x-clip">
    <header>.....</header>
</div>

@zepfietje
Copy link
Member

I don't know, @4m1n0s.
Please test it and commit if it works. :)

@zepfietje zepfietje merged commit 4e3a4e2 into filamentphp:3.x Nov 26, 2023
4 checks passed
@zepfietje
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ui
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants