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

feat: add color picker for tags #8884

Merged
merged 1 commit into from
Sep 28, 2023
Merged

feat: add color picker for tags #8884

merged 1 commit into from
Sep 28, 2023

Conversation

GretaD
Copy link
Contributor

@GretaD GretaD commented Sep 25, 2023 •
edited
Loading

Screenshot from 2023-09-28 12-19-42
Screenshot from 2023-09-28 12-20-56

fixes #5084

@GretaD GretaD self-assigned this Sep 25, 2023
GretaD
GretaD commented Sep 27, 2023
View reviewed changes
src/components/TagItem.vue Outdated Show resolved Hide resolved
@GretaD GretaD marked this pull request as ready for review September 28, 2023 10:21
@GretaD
Copy link
Contributor Author

GretaD commented Sep 28, 2023

cc @hamza221 since you're adding the delete button

@GretaD GretaD changed the title Add color picker for tags feat: add color picker for tags Sep 28, 2023
hamza221
hamza221 approved these changes Sep 28, 2023
View reviewed changes
Copy link
Contributor

@hamza221 hamza221 left a comment

Choose a reason for hiding this comment

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

Tested ✅
But this mean I have to rework my Pr 😂 😭

@hamza221
Copy link
Contributor

hamza221 commented Sep 28, 2023 •
edited
Loading

The only thing I noticed is that the color 'wheel?' and text are not aligned here
image

kesselb
kesselb reviewed Sep 28, 2023
View reviewed changes
src/components/TagModal.vue Outdated Show resolved Hide resolved
src/components/TagModal.vue Show resolved Hide resolved
src/components/TagItem.vue Outdated Show resolved Hide resolved
@GretaD
Copy link
Contributor Author

GretaD commented Sep 28, 2023

Tested ✅ But this mean I have to rework my Pr 😂 😭

do you want to get yours merged first and then i rework mine? Mine looks easier to change the position. I am fine to do that.

@hamza221
Copy link
Contributor

Tested ✅ But this mean I have to rework my Pr 😂 😭

do you want to get yours merged first and then i rework mine? Mine looks easier to change the position. I am fine to do that.

No I'll rework mine it's not much work. Thank you for offering

@GretaD
Copy link
Contributor Author

GretaD commented Sep 28, 2023

The only thing I noticed is that the color 'wheel?' and text are not aligned here image

i cannot reproduce this, super weird. Which browser and screensize do you have it misaligned?

@hamza221
Copy link
Contributor

The only thing I noticed is that the color 'wheel?' and text are not aligned here image

i cannot reproduce this, super weird. Which browser and screensize do you have it misaligned?

Chromium , 1920*932

@GretaD GretaD force-pushed the feat/add-color-picker branch from 523aa20 to ab745c1 Compare September 28, 2023 14:19
@GretaD GretaD force-pushed the feat/add-color-picker branch from ab745c1 to db38c4c Compare September 28, 2023 14:21
@GretaD GretaD merged commit 13e2107 into main Sep 28, 2023
28 checks passed
@GretaD GretaD deleted the feat/add-color-picker branch September 28, 2023 14:29
@GretaD
Copy link
Contributor Author

GretaD commented Sep 28, 2023 •
edited
Loading

hamza will double-check the misalignment after rebasing his pr if its still there, as its not reproducible by anybody else

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kesselb kesselb kesselb left review comments

@hamza221 hamza221 hamza221 approved these changes

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

Assignees

@GretaD GretaD

Labels
3. to review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

🎨🏷️ Add tag color picker
4 participants
@GretaD @hamza221 @ChristophWurst @kesselb