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.

Sign up for GitHub

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

Jump to bottom

Labels #106

Merged
merged 43 commits into from
Aug 18, 2020
Merged

Labels #106

merged 43 commits into from
Aug 18, 2020

Conversation

zhen0
Copy link
Member

@zhen0 zhen0 commented Aug 13, 2020 •
edited
Loading

PR Checklist:

  • add a short description of what's changed to the top of the CHANGELOG.md
  • add/update tests (or don't, for reasons explained below)

Describe this PR

Allows a user to add or remove flow labels in the flow details tile
Closes https://github.com/PrefectHQ/cloud-web-ui/issues/1687

@zhen0
access flow group on details tile and add labels to flow group query
288cb00
@zhen0
show flow group labels in flow details tile labels section
6018476
@zhen0
remove console logs
01071e0
@zhen0
add button to add label to flow details tile
387b7a7
@zhen0
add basic mutation
9ab7e68
@zhen0
keep exisiting labels
dcf78a7
@zhen0
add laoding and reset but needs touch up:
b6009fa
@zhen0
fix menu width
cc6deda
@zhen0
add text edit with key up enter
3bf912e
@zhen0
add chips as inner prepend and change color if chip duplicated
b341f16
@zhen0
Merge branch 'master' into labels
b411779
@zhen0
better handle long labels
c45ee55
@zhen0
neaten code and add duplicate to non-overflow
686e5cc
@zhen0
fix flashing red when 2+labels
ce34a2e
@zhen0
make sure we don't break the agent card labels
8b9c0bd
@zhen0
switch wording to remove rather than cancel
e6f36a2
@zhen0
click event to button
bca8cfc
@zhen0
rename checkinput to checklabelinput
6796046
@zhen0
remove unecessary async
b1a98fd
@zhen0
decrease label overflow size to make sure room to edit
24fe062
@zhen0
keep loading state blue
79192db
@zhen0
updwte changelog
8e4cf8f
@zhen0
merge master
0866323
@zhen0 zhen0 marked this pull request as ready for review August 13, 2020 15:52
znicholasbrown
znicholasbrown previously requested changes Aug 14, 2020
View reviewed changes
Copy link
Contributor

@znicholasbrown znicholasbrown left a comment

Choose a reason for hiding this comment

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

A few small changes in addition to the design ones discussed offline.

src/components/Label.vue Show resolved Hide resolved
src/graphql/Mutations/add-label.gql Outdated Show resolved Hide resolved
src/pages/Flow/Details-Tile.vue Outdated Show resolved Hide resolved
src/pages/Flow/Details-Tile.vue Outdated Show resolved Hide resolved
cicdw
cicdw previously requested changes Aug 17, 2020
View reviewed changes
Copy link
Member

@cicdw cicdw left a comment

Choose a reason for hiding this comment

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

I really like how this was designed - instead of a whole new Settings tab (which can be hard to discover), it's implemented directly in the Flow details tile!

I left two comments about the way we display info when the flow / flow group has no labels.

src/pages/Flow/Details-Tile.vue Outdated Show resolved Hide resolved
src/pages/Flow/Details-Tile.vue Outdated Show resolved Hide resolved
@zhen0 zhen0 dismissed stale reviews from cicdw and znicholasbrown August 18, 2020 01:23

Changes made!

cicdw
cicdw approved these changes Aug 18, 2020
View reviewed changes
Copy link
Member

@cicdw cicdw left a comment

Choose a reason for hiding this comment

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

LGTM

znicholasbrown
znicholasbrown approved these changes Aug 18, 2020
View reviewed changes
Copy link
Contributor

@znicholasbrown znicholasbrown left a comment

Choose a reason for hiding this comment

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

Thanks for the changes @zhen0, LGTM!

@znicholasbrown znicholasbrown merged commit db6a4ad into master Aug 18, 2020
@znicholasbrown znicholasbrown deleted the labels branch August 18, 2020 13:43
This was referenced Dec 14, 2021
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@znicholasbrown znicholasbrown znicholasbrown approved these changes

@cicdw cicdw cicdw approved these changes

@jlowin jlowin Awaiting requested review from jlowin jlowin is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zhen0 @cicdw @znicholasbrown