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

Grid, fix 404 taskInstance errors #26575

Merged
merged 1 commit into from
Sep 22, 2022

Conversation

pierrejeambrun
Copy link
Member

@pierrejeambrun pierrejeambrun commented Sep 21, 2022

We had a few warnings due to validateDOMNesting in the grid view (cf screenshot) -> split the table into two distinct tables with separate 'titles'. Took the opportunity to slightly improve spacing between the tables. (fixed in #26551)
image
image

We also had a few 404 errors when requesting the useTaskInstance for groups, by clicking on the details of a task group. (cf screenshot):
image

@boring-cyborg boring-cyborg bot added area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Sep 21, 2022
@pierrejeambrun pierrejeambrun changed the title Fix 404 taskInstance errors and validateDOMNesting warnings Grid, fix 404 taskInstance errors and validateDOMNesting warnings Sep 21, 2022
Copy link
Contributor

@dstandish dstandish left a comment

Choose a reason for hiding this comment

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

code looks fine but it's nice to include a screenshot of the visual change

@pierrejeambrun
Copy link
Member Author

pierrejeambrun commented Sep 22, 2022

code looks fine but it's nice to include a screenshot of the visual change

You are right. I didn't change much so I didn't bother, but here is a preview:
image

edited: @bbovenzi already fixed the dom warnings #26551, I'll edit this PR to just fixe the 404 errors.

@pierrejeambrun pierrejeambrun changed the title Grid, fix 404 taskInstance errors and validateDOMNesting warnings Grid, fix 404 taskInstance errors Sep 22, 2022
@bbovenzi
Copy link
Contributor

I like your approach of splitting it into 2 tables more. When you rebase your branch. Let's keep that.

@pierrejeambrun pierrejeambrun force-pushed the fix-grid-warnings branch 2 times, most recently from 28afb44 to 6eb2c1a Compare September 22, 2022 15:49
Copy link
Contributor

@bbovenzi bbovenzi left a comment

Choose a reason for hiding this comment

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

lgtm!

@pierrejeambrun
Copy link
Member Author

I like your approach of splitting it into 2 tables more. When you rebase your branch. Let's keep that.

Ok, i did that and updated the titles so we have something consistent with the 'Task Actions' title:
image

@dstandish
Copy link
Contributor

nice, thanks, yeah i knew the formatting was off but could not figure out what was wrong ... this looks great...

@bbovenzi bbovenzi merged commit 6f1ab37 into apache:main Sep 22, 2022
@pierrejeambrun pierrejeambrun deleted the fix-grid-warnings branch September 22, 2022 20:16
@bbovenzi bbovenzi modified the milestones: Airflow 2.4.1, Airflow 2.5.0 Sep 26, 2022
@ephraimbuddy ephraimbuddy added the type:bug-fix Changelog: Bug Fixes label Oct 18, 2022
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. area:webserver Webserver related Issues type:bug-fix Changelog: Bug Fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants