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

[Omn-190] - [Settings View] - Labels #345

Merged
merged 51 commits into from Apr 6, 2022
Merged

[Omn-190] - [Settings View] - Labels #345

merged 51 commits into from Apr 6, 2022

Conversation

gitstart-omnivore
Copy link
Contributor

Spec Checklist

  • Are Api endpoints ready and properly documented ?
  • Does the figma design fit the ticket description ?
  • Added video describing your plan of action ?

Description

This is part of the base issue Settings View

Omnivore allows users to create labels and attach them to library items. A label has a name, colour, and description , users can create labels, delete labels. They can also edit a labels color and description.

This page has a temporary version available at settings/labels

House Keeping

  • Added video describing your plan of action ?
  • Added Loom video ?
  • Is Deployment Link passing ?
  • Have you assinged PR to yourself ?
  • Is Github action passing ?
  • Updated the appropriate tag ?

Ticket link (if applicable)

Task Link

How has this been tested?

Types of changes

  • Bug fix
  • New feature
  • Refactor
  • Others

Checklist:

  • I have test my code on different browsers and devices to make sure it is cross-browser compatible (both desktop and mobile).
  • I have confirmed the payload for integrated API matches params on API docs
  • I have confirmed design matches Spec specified on Figma.

Language translation

  • Did you udpate/add a new text node ?
  • I have updated language mapping JSOn in locale

Remarks

  • I added a new package to achieve this task.
  • I have updated package.json

@vercel
Copy link

vercel bot commented Mar 30, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

omnivore-prod – ./

🔍 Inspect: https://vercel.com/omnivore/omnivore-prod/FApR6rGF7dNLmzfgCf38eaopgtw7
✅ Preview: https://omnivore-prod-git-omn-190-omnivore.vercel.app

omnivore-demo – ./

🔍 Inspect: https://vercel.com/omnivore/omnivore-demo/6Po24SzjpvRMecCZoJMU93Xr79En
✅ Preview: https://omnivore-demo-git-omn-190-omnivore.vercel.app

@gitstart-omnivore
Copy link
Contributor Author

@jacksonh this PR is ready for review, except we noticed the label update functionality wasn't implemented. We've implemented it though, but this permission error occurs when we try to update the labels, is there something we're missing?

Screenshot 2022-03-30 at 13 23 29

Please kindly take a look

@jacksonh
Copy link
Contributor

@jacksonh this PR is ready for review, except we noticed the label update functionality wasn't implemented. We've implemented it though, but this permission error occurs when we try to update the labels, is there something we're missing?

Screenshot 2022-03-30 at 13 23 29

Please kindly take a look

Ah thank you, this issue is because of the row level security attributes. I'll push a fix while reviewing.

@jacksonh
Copy link
Contributor

The text in the Create Label button isn't quite vertically centered properly. A couple extra pixels at the bottom I think.

Screen Shot 2022-03-30 at 13 31 29

@jacksonh
Copy link
Contributor

It looks like these buttons are a little too big, the figma is on the right and the implementation on the left.

Screen Shot 2022-03-30 at 13 34 15

@jacksonh
Copy link
Contributor

This dropdown needs to be a fixed width, otherwise when you are changing colours it will "jitter" as the values are changed and it resizes\

Screen Shot 2022-03-30 at 13 47 05

@jacksonh
Copy link
Contributor

Here's a quick video that shows two issues, the first is the custom color modal moving around when trying to select a custom colour. The second issue is the one I mentioned above, the box jittering around when the colour value is changed:

https://www.loom.com/share/7164c2db62c341b19cda02863c2694b0

@jacksonh
Copy link
Contributor

We should try to centre these headers over the items. Name looks ok, but description, color, actions aren't centred right

Screen Shot 2022-03-30 at 13 56 49

@jacksonh
Copy link
Contributor

One thing I notice in console is the error <button> cannot appear as a descendant of <button>. at button

@jacksonh jacksonh merged commit 589e639 into main Apr 6, 2022
@jacksonh jacksonh deleted the OMN-190 branch April 6, 2022 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants