-
Notifications
You must be signed in to change notification settings - Fork 1
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
Ephemeral Categories & Modal Confirmations #65
Conversation
…ture/ephemeral-category-creation
✅ Deploy Preview for thingswedo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Please refactor so the styling is more in line with the overall app's design
- Probably need to rethink the use of
NavLink
inAddToolInputs.tsx
- Please also delete the hover effects from
CategoriesBar.tsx
as this is resulting in unwanted UI behaviour during testing - Make sure to add a screenshot to the PR description for every UI change that has been made, atm there is only one even though several modal views have been created
- Please remove any alerts and replace them with modals
Sure. Any specifics I need to consider beyond your comment on the Modal?
No prob, sorry about that - was out of time and wanted to get the PR up |
I think try to keep buttons through the app on one line only for visual consistency - I think that the Modal text size should maybe be smaller anyway so have a look at that and maybe get team consensus on how it looks. Happy for you to ask @gurtatiLND to change the hover effects in her branch instead of doing it in yours Pls feel free to use button for the header in addtool |
I'll do this change, but I'll do that in a separate branch. Just feels odd to tweak unrelated styling in this branch.
Will do! |
…ture/ephemeral-category-creation
import modal support pending categories
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls
Co-authored-by: jackcasstlesjones <[email protected]>
Co-authored-by: jackcasstlesjones <[email protected]>
Co-authored-by: jackcasstlesjones <[email protected]>
Co-authored-by: jackcasstlesjones <[email protected]>
Co-authored-by: jackcasstlesjones <[email protected]>
What type of PR is this? (check all applicable)
Description
Screenshots
UI accessibility checklist
If your PR includes UI changes, please utilize this checklist:
Critical
andSerious
issues?Added/updated tests?
Please aim to keep the code coverage percentage at 80% and above.
Delete branch after merge?
What gif best describes this PR or how it makes you feel?