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

style the AddItem component #53

Merged
merged 8 commits into from May 26, 2022
Merged

style the AddItem component #53

merged 8 commits into from May 26, 2022

Conversation

redapy
Copy link
Collaborator

@redapy redapy commented May 24, 2022

For an example of how to fill this template out, see this Pull Request.

Description

This PR style the "Add item" page, and remove the "filter text" from the Search component

  • To make the radio buttons look better, I installed the custom-forms plugin.
  • I wrapped all the inputs inside their label because I had some issues to style them as they were.
  • Styles error and success messages

Related Issue

Closes #48
Closes #52

Type of Changes

Type
🐛 Bug fix
✨ New feature
🔨 Refactoring
💯 Add tests
🔗 Update dependencies
📜 Docs

Updates

Before

before

After

after

Testing Steps / QA Criteria

  • From your terminal, pull down this branch with git pull origin rb-style-add-item-page and check that branch out with git checkout rb-style-add-item-page
  • run npm install to install the new dependecie
  • Compare the changes with the Figma file.

Test searching the list

  • In the list page, start your search with a white space.

package.json Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented May 25, 2022

Visit the preview URL for this PR (updated for commit 5c81afc):

https://tcl-44-smart-shopping-list--pr53-rb-style-add-item-pa-aedjev8f.web.app

(expires Thu, 02 Jun 2022 12:42:43 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Copy link
Collaborator

@hellodeborahuk hellodeborahuk left a comment

Choose a reason for hiding this comment

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

Great work Reda!

src/components/AddItemForm.js Outdated Show resolved Hide resolved
src/components/AddItemForm.js Outdated Show resolved Hide resolved
@JimeBlue
Copy link
Contributor

Excellent job @redapy 🚀

@redapy
Copy link
Collaborator Author

redapy commented May 26, 2022

what do you think about the colors? I don't if green looks good, was thinking about cyan also for the success message
styles

@JimeBlue
Copy link
Contributor

Cyan would be the perfect combination for the success message IMO

@hellodeborahuk
Copy link
Collaborator

what do you think about the colors? I don't if green looks good, was thinking about cyan also for the success message

I agree with Jimena, cyan would look good. I'd just check the colour contrast of the cyan on the white. If it fails, then try a higher number like cyan-600 until it passes the contrast.

@JimeBlue
Copy link
Contributor

JimeBlue commented May 26, 2022

Great job with the changes @redapy! I love the way it looks with the messages now. This I found very instructive in relation to using aria-label or aria-labelledby https://tink.uk/the-difference-between-aria-label-and-aria-labelledby/. Also this https://www.w3.org/TR/WCAG20-TECHS/ARIA14.html

@redapy
Copy link
Collaborator Author

redapy commented May 26, 2022

Thanks for the great review. I used cyan-800 because of a11y, lees than that it will be poor contrast

@redapy redapy merged commit dbe6bc6 into main May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filter items formatting Style the "add an item" page
4 participants