light-dark-theme #15
light-dark-theme #15
Conversation
There was a problem hiding this comment.
Hey @VedmaPranav thanks for the PR!
Could you format code using make pre-commit and update the PR?
…theme * 'main' of https://github.com/OWASP/Nest: Turn server tokens off Update issue index and Nginx config Update Nginx config Make some Repository fields optional Update Repository model Set up Sentry Update CI/CD Bump openai from 1.47.0 to 1.50.2 in /backend (OWASP#18) Bump pytest-cov from 4.1.0 to 5.0.0 in /backend (OWASP#16) Bump ruff from 0.5.7 to 0.6.8 in /backend (OWASP#17) Update issue-manager config Add issue manager Update dependabot config (docker) Update dependabot config
|
@arkid15r I ran the pre-commit checks and updated the PR |
|
I mistakenly merged the main branch into this branch, I need to undo the merge. |
This reverts commit 27a2469.
So the pre-commit step of CI/CD is still failing. Could you address the pre-commit requirement issues? |
There was a problem hiding this comment.
@VedmaPranav thanks addressing the pre-commit issue
Please find my comment below and let me know your thoughts
There was a problem hiding this comment.
I noticed you didn't use the recommended bootstrap way to implement light/dark theme -- https://getbootstrap.com/docs/5.3/customize/color-modes/#enable-dark-mode
This looks like a more straight-forward solution and might not require a ton of extra CSS.
Do you think it's worth looking into the bootstrap recommended way and remove the extra complexity?
There was a problem hiding this comment.
@ arkid15r I suggest a combination of Bootstrap and some custom CSS. Bootstrap recommended adding a dark theme to the HTML tag element; however, since the page class on the body element has a background color set to #f4f6fc we would see something as the following screenshot when dark theme is applied.
So, we must set the background color to None when the dark theme is applied. Also, the tooltips/github icons in the dark mode have a white border; we can remove this border through some custom CSS.
app.css file also contains some CSS related to the light-dark toggle slider. Switching this slider to a button or something similar will lose the aesthetic of a toggle slider.
The following is a screenshot of how the page would look with the changes I suggested. let me know your thoughts
There was a problem hiding this comment.
Can we use font-awesome icons for the style consistency?
There was a problem hiding this comment.
changed the icons to font-awesome icons
This PR addresses Issue #3
Screenshot of the change:
Please feel free to suggest different styling, or any other feedback