Improve Dag run and Task Instance notes #51859
Conversation
jscheffl
requested review from
bbovenzi,
pierrejeambrun,
ryanahamilton and
shubhamraj-git
as code owners
bbovenzi
changed the title
|
This is better. I still don't like the Tabs moving up and down. I might pull your branch down and play around with the UX |
There was a problem hiding this comment.
Thanks for the follow up. I think this can be a good alternative compare to what we currently have on main.
I see that you also figured out that plenty of images would just make the details tab unusable without this PR. (I was about to create an issue about it).
Not approving to let Brent time to investigate an alternate solution.
| import EditableMarkdownArea from "./EditableMarkdownArea"; | ||
| import { Accordion } from "./ui/Accordion"; | ||
|
|
||
| const EditableMarkdownNotes = ({ |
There was a problem hiding this comment.
I don't think the name of the component is super descriptive. (vs EditableMarkdownArea)
Because it leaves us with Area vs Notes, impossible to guess who is collapsible, who's not, what's the difference.
|
Yeah, thanks for the feedback. Feel free to pull it down and experiment with it. I am not sure where but certain there mgiht be ready-made components in the React ecosystem which would allow an expansion on demand besides the accordion. So I think both commeonts from you are valid and just wanted to signal that I am okay to improve... just my React skills ight be limited to the level like in this PR :-D Yeah and of course we can also leave the maxHeight to a static limit as an increment... I assume the example with the images is theoretically possible but a very rare/extreme case. Usually I'd expect a few words or notes and as the field is limited to 1000 chars... ~300px might be a realistic size and scrollbars might be also sufficient not to make it overly complex. So yeah, constructive ideas welcome, no pressure but would be good to make it "pretty" before 3.1. |
|
Unrelated to this PR, existed before, created a separate issue #53085 |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. |
github-actions
bot
added
the
stale
|
Closing in favor of #54163 |
3 participants
Follow-up of #51764
As @pierrejeambrun and @bbovenzi had some feedback, this adds an accordion and limits the maximum space used, add an option to hide notes.
Better like this?
dag_run_notes2-2025-06-17_22.47.56.mp4