Skip to content

Remove unneeded form visibility scrollbars #19993

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cconard96
Copy link
Contributor

Checklist before requesting a review

  • I have read the CONTRIBUTING document.
  • I have performed a self-review of my code.

Description

Remove another forced scrollbar display in form editor

@trasher trasher requested review from orthagh and cedric-anne June 13, 2025 06:43
@orthagh orthagh requested a review from ccailly June 13, 2025 07:07
@orthagh
Copy link
Contributor

orthagh commented Jun 13, 2025

This one requires a look from either @ccailly or @AdrienClairembault

Copy link
Member

@ccailly ccailly left a comment

Choose a reason for hiding this comment

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

In both cases, I am unable to reproduce behavior that uses a scrollbar.
If the visibility dropdown takes up more space than is available, the scrollbar appears at the level of the entire form editor rather than on the dropdown.

In any case, there is some minor styling work to be done on the condition dropdowns.
If the target name is too long, you end up with this kind of field for entering the value (the field width is too small to see even a single character):
image

@cconard96
Copy link
Contributor Author

Well removing the forced display of scrollbars and changing it to be automatic is legitimate, but there is still issues with the card never overflowing. It doesn't have a maximum width, and instead continues growing to fit-content and so it itself overflows the form editor.

@cedric-anne cedric-anne requested a review from ccailly June 17, 2025 09:14
min-width: 700px;
max-width: 700px;
Copy link
Member

Choose a reason for hiding this comment

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

I think the min-width is necessary; we don't want the dropdown to be too small.
700px for the min-width seems fine to me.
In my opinion, the max-width should correspond to the available space in the form container.

Copy link
Contributor Author

@cconard96 cconard96 Jun 17, 2025

Choose a reason for hiding this comment

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

That isn't how it works in practice though. The dropdown panel's scroll container is the editor (.form-editor) which has overflow: auto. It will continue to grow infinitely and overflow the editor element.

The panel will never be too small because it contains a heading and 3 buttons which would keep the panel from shrinking too much. Still, we can also add a smaller min width.

@cedric-anne cedric-anne requested a review from ccailly June 23, 2025 10:13
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.

4 participants