touchup(drop down menu): applied icons and background colors for UX #633
Conversation
✅ Deploy Preview for velvety-vacherin-4193fb ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for ameliorate-docs canceled.
|
There was a problem hiding this comment.
Nice work so far! I couldn't tell from your commit that you're new to open source & TypeScript 🙂 .
Re: the options under "Change node type": good catch, I forgot about that! I'd be happy to have you make the same changes there as well if you're up for it, otherwise I can create a separate ticket.
Also:
- the commit type
styleis intended for code style changes; heretouchupis more apt - GitHub automatically links issues with syntax like
#629- you've left the PR description with square brackets, so it isn't linked
cleaning up coder comments. Co-authored-by: Joel Keyser <[email protected]>
bot will fix my contribution credits! Co-authored-by: Joel Keyser <[email protected]>
Changing to Tailwind inline styling Co-authored-by: Joel Keyser <[email protected]>
|
I would be happy to take on the "Change Node Types" task discussed. Please create a separate ticket and assign it to me if you can! Let me know if you want to swap background colors for icon colors too. I will be hiking and camping fri-sun and far far away from the computer, but I will check back first thing monday. Thanks for all your quick and insightful responses! I appreciate the guidance and patience. |
rdubbi
changed the title
Created #636 - it seems that I can't assign you unless you comment on that issue though.
No rush! Have fun 🙂 |
Per suggestion, removed to allow bot to make list my contribution automatically.
Incorporated code to color the background of the icon instead of the whole menu item, as discussed.
|
It is my first time updating a PR in github after submission, I hope I did everything right! I changed the background colors as discussed and removed my manual contribution to allow the bot to redo it. I will wait for this to be merged before pulling down fresh code to start on #636 . |
There was a problem hiding this comment.
Looks good! Some failing checks but it's ok:
- commitlint: failing because commits beyond the first don't match the required format; but I squash all commits on merge, so only the first one matters anyway
- lint: some of the code wasn't indented per the prettier standard - I just ran
npm run check-pretty -- --writeto fix this and pushed that up. (this should be running on file save, you may need setup for that described here Feat: #482 when Reset topic clicked it should delete comments too #580 (comment)). - e2e: this is a bug in my e2e check 😅 I'll fix this when I get the chance... but we can ignore for now
I'll merge this! Thanks for your contribution 🙂.
Before this change:
|
@all-contributors please add @rdubbi for code and design |
|
I've put up a pull request to add @rdubbi! 🎉 |
Closes #629
Description of changes
All changes were made in AddNodeMenuItems.tsx. I imported useTheme. I built upon shownNodeTypes.map so that it carried over the icons from node.ts and the background colors from theme.ts.
Additional context
General "New Guy, please help" Disclaimer:
I graduated from a coding bootcamp and have spent months applying to jobs. I thought contributing to some open source projects would be a great opportunity to grow as a software engineer, make the world a better place, and bolster my resume. This is my very first open source contribution, and my very first time using TypeScript. All of this to say, I tried my best to follow all of the contributor guidelines and "clean code" best practices, so if there are any glaring mistakes or deviations from contributor best practices, I promise it is ignorance, not sloppiness, and I wholeheartedly welcome any and all feedback!
Future Changes:
From the directions in issue 629, it seemed to me that the change was only to be applied to the "Add Node" drop down options, but I noticed while playing around with the software that an identical set of drop down options can be found under the "Change Node Type" drop down when right clicking on the side bare instead of on the whiteboard. Let me know if you want me to extend the same changes to that other drop down! I have attached a photo of what I am referring to.