-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Enhance note button visibility with highlighting when notes exist #54163
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
Conversation
- Add yellow solid button styling when DAG run or task instance has a note - Fix state synchronization when switching between runs to maintain proper highlighting - Improve user experience by making existing notes more discoverable in the UI
|
Oh, after I made earlier the PR #51764 which was debated and later reverted (partially) by @pierrejeambrun this is one alternative we can have a look. I agreed with Pierre that the visibility of the note and the results in layout might need to be discussed. Unfortuantely I was distracted the last week and had no focus on creating an proposal. This PR could be one option to highlight that notes exist. Whereas still you would need to click to see it. I am still a bit in favor of directly displaying but also my proposal in #51859 was not rated positive. |
|
@jscheffl, reviewing your PR, I think having the note directly displayed is much better! It helps identify runs with one click where current approach is 2 clicks. I have an issue where I want to add some notes/markers to be able to easily identify runs in a sea of dag runs. Having to click through every note button adds friction. If we think this will take time, I would prefer my PR be considered as it addresses the visibility issue of the current notes atleast. |
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.
I think there is two things that we can improve for note visibility:
-
Find a way to identify quickly which dag/ti contains a note across multiple runs/tis. In AF3 we had a small transparency marker on the grid to indicate that and I think it was a smart way of achieving it.
-
Find a way to display the note in a more natural way and drag attention of users to the note, so they do not miss it before performing any action on the TI or DagRun. |
For 1) I tend to think that the way we did it in AF2 was actually good, but I'm open to other suggestions.
For 2) I think that button coloring or some 'notification' marker on the button could drag user attention that there is a note here. Embedding the note directly in the main details tab is tricky for the reason mentioned in Jens PR. And if we end up putting it in a dedicated tab, it's still extra clicks to get there. For instance maybe a simple 'notification' indicator on the button, similarly to what github is doing could be nice:
![]()
|
I don't think that the whole button changing color fits supper well into the current UI, because buttons have consistent colors throughout the UI and having just one button yellow seems weird. Also for accessibility reason (for instance people that do not see colors very well), we try to avoid relying on only colors to implement a feature, another visual indicator could help. |
- Add blue notification dot on top-right corner of Add Note button when notes exist - Remove yellow color highlighting and solid variant changes from button - Use Chakra UI design tokens for consistent sizing - Maintains subtle visual indicator while preserving original button appearance
|
@pierrejeambrun , I updated this to be similar to github, with a small blue notification icon
|
Fair enough, I updated the PR to align with what github is doing for notifications |
pierrejeambrun
left a comment
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.
That looks cool 🎉
A few last adjustments to make.
airflow-core/src/airflow/ui/src/components/EditableMarkdownButton.tsx
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/ui/src/components/EditableMarkdownButton.tsx
Outdated
Show resolved
Hide resolved
|
Can you also remove or update the PR description ? (screenshots) |
…ton.tsx Co-authored-by: Pierre Jeambrun <pierrejbrun@gmail.com>
…ton.tsx Co-authored-by: Pierre Jeambrun <pierrejbrun@gmail.com>
Remove useEffect hooks that were synchronizing note state and replace with direct computation using hasContent pattern. This eliminates extra renders and fixes notification dot persistence issue when switching between runs.
|
@pierrejeambrun , I've incorporated all your suggestions. Please let me know if you think we need anything else |
@bbovenzi , Use this on the Add Note button you mean? Or do you mean like a translucent overlay ontop of the dag run column like how we had for airflow 2? ( like the image below?) |
Just the button. With icons added to the grid view we removed the note indicator. It was too much going on in a small space. |
Replace static FiMessageSquare icon with conditional PiNoteBlankLight/PiNoteLight icons based on note content. Shows PiNoteBlankLight when no note exists and PiNoteLight when a note is present, providing clearer visual indication of note status.
Done! the note icon is now using PiNote icons |
|
@pierrejeambrun, I tried your suggestion on relying on the hasConten. When you try to toggle between multiple tasks with/without notes it still seems to carry over the notification dot even when there isn't a note and is needing a page refresh to render correctly. The useEffect seems to be the only reliable way at least in my experiments. Do you think you can contribute a tweak here which I can test out? else if you think the useEffect here is acceptable let me know if we can merge. |
jscheffl
left a comment
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.
Looks good to me. I assume just there is a bug with the note popup (not this PR).
note_text_bug-2025-08-16_12.16.41.mp4
Still I'd favor having the note presented w/o the need to have a click for a popup to have direct visibility (especially when scrolling through a number of tasks/dags... but my alternative proposal was not getting traction until now :-(
I hear you, I prefer the note showing directly as well. Let me brainstorm on this on the side (in another pr) to see if we can do something similar to your original PR in a way that it doesn't disturb the layout. Meanwhile if these current changes are good with everyone lets merge them in. |
bbovenzi
left a comment
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.
I agree. We can keep thinking of a UX that shows the notes without affecting the formatting of the whole page. But this PR is definitely an improvement
…ache#54163) * Enhance note button visibility with highlighting when notes exist - Add yellow solid button styling when DAG run or task instance has a note - Fix state synchronization when switching between runs to maintain proper highlighting - Improve user experience by making existing notes more discoverable in the UI * Replace note button color change with notification dot indicator - Add blue notification dot on top-right corner of Add Note button when notes exist - Remove yellow color highlighting and solid variant changes from button - Use Chakra UI design tokens for consistent sizing - Maintains subtle visual indicator while preserving original button appearance * Update airflow-core/src/airflow/ui/src/components/EditableMarkdownButton.tsx Co-authored-by: Pierre Jeambrun <pierrejbrun@gmail.com> * Update airflow-core/src/airflow/ui/src/components/EditableMarkdownButton.tsx Co-authored-by: Pierre Jeambrun <pierrejbrun@gmail.com> * Refactor Header components to remove unnecessary useEffect hooks Remove useEffect hooks that were synchronizing note state and replace with direct computation using hasContent pattern. This eliminates extra renders and fixes notification dot persistence issue when switching between runs. * Fix static checks * Use conditional note icons in EditableMarkdownButton Replace static FiMessageSquare icon with conditional PiNoteBlankLight/PiNoteLight icons based on note content. Shows PiNoteBlankLight when no note exists and PiNoteLight when a note is present, providing clearer visual indication of note status. --------- Co-authored-by: Pierre Jeambrun <pierrejbrun@gmail.com>








Without note:

With Note:
