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

Remove minimum height on elements in Flexible Form #46591

Merged

Conversation

jscheffl
Copy link
Contributor

@jscheffl jscheffl commented Feb 8, 2025

I noticed that the flexible trigger form always used minimum 80px as space per form row, even if content is less. This occupies a too much screen space for simple forms.
Root cause is that the labels are receiving a minimum height of 80px if no flex basis is defined as style. This PR fixes the layout bug.

Before:
image

After / with this PR (incl. review comment fixes):
image

FYI @shubhamraj-git

@boring-cyborg boring-cyborg bot added the area:UI Related to UI/UX. For Frontend Developers. label Feb 8, 2025
Copy link
Contributor

@shubhamraj-git shubhamraj-git left a comment

Choose a reason for hiding this comment

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

Seems the upper margin and the lower margin of each elements are not same, a bit weird, can we have it in sync?

@jscheffl
Copy link
Contributor Author

jscheffl commented Feb 9, 2025

Seems the upper margin and the lower margin of each elements are not same, a bit weird, can we have it in sync?

Good catch. I just checked the overall height and the label... but there seems to be a bit of un-aligned margin onthe filed in the right column... will update...

@jscheffl jscheffl force-pushed the bugfix/fix-trigger-form-minimum-height branch from 37b6bf1 to 05912d6 Compare February 9, 2025 21:16
@jscheffl
Copy link
Contributor Author

jscheffl commented Feb 9, 2025

Seems the upper margin and the lower margin of each elements are not same, a bit weird, can we have it in sync?

Good catch. I just checked the overall height and the label... but there seems to be a bit of un-aligned margin onthe filed in the right column... will update...

Okay, fixed the layout glitch. I first thought it is a margin problem and then needed to hunt-down the root a bit. The problem was a condition that the helper text was not conditionally be rendered but always also as an empty element. That space was causing an un-alignment.
Therefore I needed to adjust the param spec interface as the description is provided by API as null and due to eslint all was assumed to be undefined. But it is not the same. Needed some eslint-ignore to correctly use and itnerpret a null value from description as this is always passed with null even if empty.
Side effect is that the condition for markdown actually was broken, now the markdown is happyly and correctly displayed.

@pierrejeambrun pierrejeambrun merged commit f4165fe into apache:main Feb 12, 2025
35 checks passed
ambika-garg pushed a commit to ambika-garg/airflow that referenced this pull request Feb 17, 2025
* Remove minimum height on elements in Flexible Form

* Fix helper description consistency in display
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:UI Related to UI/UX. For Frontend Developers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants