-
Notifications
You must be signed in to change notification settings - Fork 338
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
feat: Bootstrap 5 #1378
feat: Bootstrap 5 #1378
Conversation
LGTM, thanks for this! 🚀 |
* origin/main: fix: Prettier ignore generated themes, as they aren't written by humans feat: Badge-ify NSFW and removed by mod title info
* origin/main: fix(post): Fix missing labels on block/report buttons in new dropdown fix: Add compiled theme stylesheets fix: Fix too-intense hr color between posts
Low priority, but the tagline items in the admin panel have a strange hover background: Untitled.mov |
@alectrocute These already look this way, and are fixed in #1382. Thank you for all the feedback! Even in |
Please ignore any feedback that isn't related to/shouldn't be addressed in this PR. Thanks @jsit, good work as always! |
Thanks a ton for doing this! This should have some priority as the merge conflicts will start to pile up if we do too much work in other areas. @SleeplessOne1917 Do you think this should come before or after |
@dessalines I think before 0.18 would be better. |
@dessalines @SleeplessOne1917 I'll work on getting these final issues sorted out. Thanks! |
* origin/main: fix: Litely Red was importing the wrong vars Fixing missing class for language select. Updating translations.
Fixed.
This is in
Fixed.
Fixed.
Fixed. |
Personally I think it might be better to wait. This PR is pretty young for its size, 0.18 is already in RC status, and I’d hate to saddle any instances with any undetected regressions this might cause until the next release. This isn’t solving much for the end user, just some dependency maintenance. However you’re right about the merge conflicts potentially becoming a big problem, so it should probably be merged very shortly after 0.18 is cut at least. But if it would be more work to wait, I understand that too. It might also not be appropriate for a point release (0.18.1) so that makes sense too. |
I'll test this out locally rn. Once we hit It will probably be a bit on |
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.
I tested it, and did a quick pass of some of the components, and everything looks good to me.
Yall have been making my clunky UI into something so much better :) Thanks a ton.
I agree with sleepless and merge this as is, and any other regressions we find can be fixed in smaller more disectable PRs.
…-a11y * origin/main: (47 commits) Fix mobile navbar bug (LemmyNet#1428) feat: Hide 'comments' in post listing comments button; icon and title text is clear feat: Drop dependency for tsconfig-paths-webpack-plugin fix main.css vars (LemmyNet#1424) forgot an import add tsconfigpathsplugin to resolve component classes v2 make suggested changes v0.18.0-rc.3 make suggested changes v0.18.0-beta.9 feat: Bootstrap 5 (LemmyNet#1378) Add scripts to make managing translations easier (LemmyNet#1414) Fix redirect after successful password reset fix: Shrink and normalize some post action button colors and sizes update imports export default everything, will fix type errors next fix missing imports fix: Litely Red was importing the wrong vars fix: Always show advanced post buttons dropdown ...
Fully remove Bootstrap 4 dependency in favor of Bootstrap 5
I should note that this changes the main green color in the Litely theme; Bootstrap 5 automatically generates foreground colors for given background colors, including with a set minimum contrast level.
I set the minimum contrast level to 3 (which passes WCAG AA for "large" text) -- the Bootstrap 5 default is a contrast level of 4.5, which would require a much darker green to handle. (The current white-on-green is only 2.23.)
Should probably offer a high-contrast theme that gets automatically selected for the
prefers-contrast: more
media query.cc @fheft