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

Make Scroll within Panel inherit bottom rounded corners #1358

Merged
merged 1 commit into from
Nov 9, 2023

Conversation

acelaya
Copy link
Contributor

@acelaya acelaya commented Nov 9, 2023

This PR addresses a slight visual glitch happening when rendering Panels without buttons, in which the bottom corners were rendered without roundiness.

Context

the Panel component is a wrapper around Card, which conditionally renders <CardContent /> as the last children if some buttons have been provided, and can also wrap the panel content with <Scroll /> if scrollable prop is true.

For simplicity, the default value for scrollable is true, so that you don't need to remember adding it and it just works if the content is too long.

During the development of hypothesis/client#5938, we found some Panels and Dialogs where bottom corners were not rendered as rounded, because they didn't have buttons. It was worked aroundt by setting scrollable={false}, as this was not needed: hypothesis/client#5938 (comment)

With these changes, that workaround would not be needed anymore.

Testing steps

The pattern library included two similar examples of scrollable panels. I have edited one of them so that it does not include buttons, in order to verify rounded corners work.

  1. Check out this branch and go to http://localhost:4001/layout-panel.
  2. Use the browser's search capabilities to search for "scrollable".
  3. Check both examples render bottom corners as rounded.

@acelaya acelaya requested a review from robertknight November 9, 2023 14:14
@acelaya acelaya merged commit 52f45cb into main Nov 9, 2023
2 checks passed
@acelaya acelaya deleted the rounded-scrollable-panel branch November 9, 2023 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants