search

nusmodifications / nusmods

🏫 Official course planning platform for National University of Singapore.
https://nusmods.com
MIT License
558 stars 270 forks source link

Add close button in planner settings modal #3702

Closed jloh02 closed 3 months ago

jloh02 commented 3 months ago

Context

Fix #3699

Implementation

vercel[bot] commented 3 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nusmods-export ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 1, 2024 10:00am
nusmods-website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 1, 2024 10:00am
vercel[bot] commented 3 months ago

@jloh02 is attempting to deploy a commit to a Personal Account owned by @nusmodifications on Vercel.

@nusmodifications first needs to authorize it.

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 53.58%. Comparing base (5774b51) to head (cfcf96f).

Files Patch % Lines
...iews/planner/PlannerContainer/PlannerContainer.tsx 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #3702 +/- ## ======================================= Coverage 53.58% 53.58% ======================================= Files 273 273 Lines 5983 5983 Branches 1429 1429 ======================================= Hits 3206 3206 Misses 2777 2777 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ZhangYiJiang commented 3 months ago

The approach is not wrong, but this means that the close button always takes up a row on its own. The close button already has styles so it is on the right side of the modal (using floats). You can also adjust the positioning after that using negative margin top and right.

https://github.com/nusmodifications/nusmods/assets/445650/7f502ed4-c27c-4940-b4f2-667d7a8368cd