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

Adjust JSX element depth from 5 to 7 #45380

Merged
merged 1 commit into from
Jan 3, 2025

Conversation

jscheffl
Copy link
Contributor

@jscheffl jscheffl commented Jan 3, 2025

After merge of PR #45273 the canary failed in UI static checks.

Error run is: https://github.com/apache/airflow/actions/runs/12598486200/job/35113887885

Reason is JSX validation which checks element depth <5 - which in my view does not make sense if we render tables with a few elements that unfortunately easily reach a level of 7 - cutting sub-components does not smell resonable.

Therefore with this PR I propose to change the accepted JSX element depth to 7.

Alternatively we would need to revert and rework the PR #45273 and cut more components to the UI.

@boring-cyborg boring-cyborg bot added the area:UI Related to UI/UX. For Frontend Developers. label Jan 3, 2025
@jscheffl jscheffl force-pushed the bugfix/adjust-eslint-jsx-depth-to-7 branch from 0000c69 to 1c7bc7c Compare January 3, 2025 16:18
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.

+1 Much needed, was often facing this issue.
Thanks for the fix.

@jscheffl
Copy link
Contributor Author

jscheffl commented Jan 3, 2025

Oh, there are more problems with curent code on main. Seems we need to revert the PR #45273 and make it proper again...

@shubhamraj-git
Copy link
Contributor

Oh, thats strange, static check didn't failed for that PR.

@jscheffl
Copy link
Contributor Author

jscheffl commented Jan 3, 2025

There was a bug in CI and checks were skipped :-(

@shubhamraj-git
Copy link
Contributor

Ohh, we need to have it fix, do we have any issue number to track that?

@jscheffl jscheffl force-pushed the bugfix/adjust-eslint-jsx-depth-to-7 branch from 1c7bc7c to 5657955 Compare January 3, 2025 16:30
@jscheffl
Copy link
Contributor Author

jscheffl commented Jan 3, 2025

Now green!

@jscheffl jscheffl merged commit cc1c401 into apache:main Jan 3, 2025
35 checks passed
@potiuk
Copy link
Member

potiuk commented Jan 3, 2025

Nice!

LefterisXefteris pushed a commit to LefterisXefteris/airflow that referenced this pull request Jan 5, 2025
agupta01 pushed a commit to agupta01/airflow that referenced this pull request Jan 6, 2025
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.

5 participants