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

feat: display labels on execution detail page #882

Merged

Conversation

blaketastic2
Copy link
Contributor

TL;DR

This PR adds the spec labels to the execution details page

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

image

Signed-off-by: Blake Jackson <blake@BB8.local>
@blaketastic2 blaketastic2 force-pushed the blake/display-labels-execution branch from 87b662d to 41e0139 Compare July 9, 2024 18:57
@JamesHoover
Copy link

This is great, thank you. A few items I would like to see:

  1. Updated color, yellow is too prominent, update to a secondary color like grey.

  2. Overflow states:

  • what happens when a single label is very long?
  • what happens when you have 3+ rows of labels?
  1. Empty state, what happens when there are no labels?

  2. Interactions, are these intended to be clickable or copyable?

@blaketastic2
Copy link
Contributor Author

blaketastic2 commented Jul 9, 2024

Here's what it looks like with more(both longer and more rows)

image

@blaketastic2
Copy link
Contributor Author

blaketastic2 commented Jul 9, 2024

Here's an updated version with the info color set on the Chip.

image

@blaketastic2
Copy link
Contributor Author

Empty state, the column doesn't show. Would you prefer the --- behaviour instead?

Also, there is no functionality on the Chips. If someone wants to add something later(like copy to clipboard), I think it should be trivial. Do you think this is ok?

@JamesHoover
Copy link

Empty state, the column doesn't show. Would you prefer the --- behaviour instead?

Also, there is no functionality on the Chips. If someone wants to add something later(like copy to clipboard), I think it should be trivial. Do you think this is ok?

Yes, I would prefer the --- behavior for the empty state for consistency and discovery. Otherwise this looks good to me. Thank you for your contribution!

Signed-off-by: Blake Jackson <blake@BB8.local>
@blaketastic2
Copy link
Contributor Author

Ok, done.

image

@jsonporter jsonporter merged commit 7636e32 into flyteorg:master Jul 10, 2024
10 checks passed
@flyte-bot
Copy link
Collaborator

🎉 This PR is included in version 1.17.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants