mlool / workday-calendar-extension

MIT License
60 stars 24 forks source link

refactor: add unified modal layer #121

Closed ThatNerdSquared closed 2 months ago

ThatNerdSquared commented 2 months ago

What Issue does this PR resolve? (Link to GitHub Issue, approved features and bugs will be given priority)

This PR implements the following...

bugfixes/enhancements:

code cleanups:

Please provide a video demo below, or a screenshot and description of the change.

https://github.com/mlool/workday-calendar-extension/assets/72814106/a8b9b36e-f949-4b33-b80e-6e21e11a3c6c

Tag reviewers for the PR below.

@mlool NOTE: SectionInfoBody.tsx is really just SectionPopup.tsx with some changes, and renamed - unfortunately there are enough differences that git thinks they're completely different files. Sorry about that.

jaskirat-gill commented 2 months ago

Couple nitpicks while we're refactoring

  1. I know its unrelated but Can we add the same padding to the discord button as the rest of the settings? image

  2. As per the conversation we had on discord, I think everyone liked the idea of having the linked text in the section popup to say something like "<< View Course Page >>" Instead of having the code twice

Otherwise, i looked over everything and it looks great. Didnt observe any bugs or anything

ThatNerdSquared commented 2 months ago

Updated the section popup:

Screenshot 2024-06-17 at 13 51 11

Not sure if it makes sense to have it in such large font compared to the course name - I think this info view could use a little styling cleanup but that will have to wait for another PR.

The settings padding is, unfortunately, inconsistent across elements. I've added some to the Discord button for now but that whole pane could use a styling cleanup at some point. Screenshot 2024-06-17 at 13 51 24

Also, I've included an updated yarn.lock, as yarn tells me to run yarn install when I build the extension for the first time. Let me know if this should be reverted.