-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
Always draw borders if task instance state is null or undefined #18033
Always draw borders if task instance state is null or undefined #18033
Conversation
bfbad06
to
9b36c09
Compare
9b36c09
to
51a2a00
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Would you mind adding some before and after screenshots for other reviewers to see the UI changes?
@bbovenzi Added the screenshots above! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease. |
@bbovenzi Thanks! Quick question - do you know why attribute values are written as integers when standalone but as strings when part of a ternary operator? Is it just a style preference? For example: airflow/airflow/www/static/js/tree.js Lines 279 to 281 in 51a2a00
airflow/airflow/www/static/js/tree.js Lines 284 to 286 in 51a2a00
|
@edwardwang888 Honestly, I don't know. It doesn't make a difference if its a string or an integer. |
Task instances of manual runs are usually drawn without borders. However, when a task instance has
null
state, it blends into the background without a border. In this case, drawing a border makes sense, but draw it at reduced opacity to distinguish it from scheduled runs.Task instances with
undefined
state were previously always drawn with full-opacity borders becauseexternal_trigger
for these task instances wasundefined
. This has been fixed, so these task instances are now drawn with borders and the proper opacity, depending on whether they are part of manual or scheduled runs.Common task instance properties are now defined in a single place, and
external_trigger
is included as a common property so that it will not beundefined
as described above.Fixes #16877.
Screenshots:
2. Clear all task instances in manual run. This results in task instances with
null
state.3. Clear all task instances in the scheduled run
4. Add new tasks to the DAG. This results in task instances with
undefined
state:5. Mouseover effect of the scheduled run
Before:
After:
6. Mouseover effect of the manual run
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.