Skip to content
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

Merged
merged 47 commits into from
Jun 20, 2023
Merged

feat: Bootstrap 5 #1378

merged 47 commits into from
Jun 20, 2023

Conversation

jsit
Copy link
Contributor

@jsit jsit commented Jun 18, 2023

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

@jsit jsit marked this pull request as ready for review June 18, 2023 21:37
@jsit jsit marked this pull request as draft June 18, 2023 21:58
@jsit jsit marked this pull request as ready for review June 18, 2023 22:58
@ayan4m1
Copy link
Contributor

ayan4m1 commented Jun 19, 2023

LGTM, thanks for this! 🚀

jsit and others added 6 commits June 18, 2023 23:15
* 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
@alectrocute
Copy link
Contributor

Low priority, but the tagline items in the admin panel have a strange hover background:

Untitled.mov

@jsit
Copy link
Contributor Author

jsit commented Jun 19, 2023

Tabs have a little bit of bottom border radius wonkiness:

@alectrocute These already look this way, and are fixed in #1382.

Thank you for all the feedback! Even in main there are some weird visual quirks, so it would be helpful if only regressions were pointed out here.

@alectrocute
Copy link
Contributor

Please ignore any feedback that isn't related to/shouldn't be addressed in this PR. Thanks @jsit, good work as always!

@dessalines
Copy link
Member

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 0.18.0?

@SleeplessOne1917
Copy link
Member

@dessalines I think before 0.18 would be better.

@jsit
Copy link
Contributor Author

jsit commented Jun 20, 2023

@dessalines @SleeplessOne1917 I'll work on getting these final issues sorted out. Thanks!

@jsit
Copy link
Contributor Author

jsit commented Jun 20, 2023

@alectrocute

Low priority, but the tagline items in the admin panel have a strange hover background:

main does this too, not a regression.

At some point, our search form became a lot more squished in the corner. Would be nice to use all available space.

Fixed.

Tabs have a little bit of bottom border radius wonkiness:

This is in main; will be fixed in #1382.

Did this table background color change?

Fixed.

One more thing – Noticed some missing right margin on various items on Modlog:

Fixed.

input-group has some side effects.

Fixed.

@jsit
Copy link
Contributor Author

jsit commented Jun 20, 2023

Do you think this should come before or after 0.18.0?

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.

@dessalines
Copy link
Member

I'll test this out locally rn.

Once we hit 0.18.0, we should be able to do much faster releases. These bigger PRs always tend to be blockers, and then fixing the merge conflicts in them quickly becomes a pain, so I agree that it's good to get them in ASAP.

It will probably be a bit on 0.18.0 anyway, as we're finding some back-end issues that need fixing. So I'm good merging this if most things test out okay.

Copy link
Member

@dessalines dessalines left a 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.

@dessalines dessalines enabled auto-merge (squash) June 20, 2023 11:40
@dessalines dessalines merged commit c77086a into LemmyNet:main Jun 20, 2023
@jsit jsit deleted the feat/bootstrap-5 branch June 20, 2023 13:33
jsit added a commit to jsit/lemmy-ui that referenced this pull request Jun 21, 2023
…-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
  ...
@Artoria2e5 Artoria2e5 mentioned this pull request Jul 1, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants