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.

Sign up for GitHub

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

Jump to bottom

feat(theme): Vaporwave #1682

Merged
merged 9 commits into from
Jul 4, 2023
Merged

feat(theme): Vaporwave #1682

merged 9 commits into from
Jul 4, 2023

Conversation

SleeplessOne1917
Copy link
Member

Bringing back yet another old theme.

Vaporwave Light:
image
image
image
image

Vaporwave Dark:
image
image
image

@jsit
Copy link
Contributor

jsit commented Jun 29, 2023

Now that all these themes are coming in it seems like a good time to mention my concern about using secondary as an "accent color."

In the default Bootstrap color palette, this is a gray color, so in a non-themed Bootstrap setup, developers would expect text-secondary to be gray, not some kind of bright blue.

I think it might be better to assign things like $teal-300 or something to the UI variables like $border-color, $link-color found in node_modules/bootstrap/scss/_variables.scss.

It seems to me that in many of the places where we use -secondary to get that green color of the default theme, instead, green should be the primary color, and we should be referring to -primary in our classes.

So in the case of this theme, that blue would be -primary. This would require rewriting a couple theme vars and a bunch of classes in the JSX.

Thoughts?

@jsit jsit changed the title Vaporwave theme: Vaporwave Jun 29, 2023
@jsit jsit changed the title theme: Vaporwave feat(theme): Vaporwave Jun 29, 2023
@SleeplessOne1917
Copy link
Member Author

Now that all these themes are coming in it seems like a good time to mention my concern about using secondary as an "accent color."

That's how I've been thinking of primary versus secondary color.

In the default Bootstrap color palette, this is a gray color, so in a non-themed Bootstrap setup, developers would expect text-secondary to be gray, not some kind of bright blue.

Are there any other official/popular bootstrap themes to reference to see what they're doing? It's hard to generalize this based on a sample size of 1. If secondary ends up being consistently used for more dull colors, then we should change our themes to accommodate that.

@jsit
Copy link
Contributor

jsit commented Jun 29, 2023

No, I don’t really know any other Bootstrap themes at all, it is just a sample size of one. If it feels fine to others then it won’t bug me.

@dessalines
Copy link
Member

@SleeplessOne1917 is this taken from the most recent / bootstrap 5 version of bootswatch? IE this one: https://bootswatch.com/vapor/

@SleeplessOne1917
Copy link
Member Author

is this taken from the most recent / bootstrap 5 version of bootswatch?

No. It's the theme from Lemmy 0.16.7 with some tweaks to get it working with the recent UI changes.

@dessalines
Copy link
Member

mmk, probably fine as is then.

@dessalines dessalines enabled auto-merge (squash) June 29, 2023 11:59
@dessalines dessalines merged commit f6cbc89 into main Jul 4, 2023
@jsit jsit mentioned this pull request Jul 4, 2023
Merged
jsit added a commit to jsit/lemmy-ui that referenced this pull request Jul 4, 2023
@jsit
Merge remote-tracking branch 'lemmy/main' into fix/fix-toggle-radio-f…
e5b5c08 
…ocus-1772

* lemmy/main: (25 commits)
  v0.18.1-rc.10
  Attempt to fix inability to logout from some instances (subdomains) (LemmyNet#1809)
  feat(theme): Vaporwave (LemmyNet#1682)
  fix: Revert smaller text size (LemmyNet#1795)
  Updated the regex for isAuthPath to reduce false positive hits (LemmyNet#1806)
  fix: Add focus border to markdown toolbar buttons
  fix: Add data-bs-theme attribute for user dark/light modes (LemmyNet#1782)
  v0.18.1-rc.9
  fix: Fix comment collapse and vote buttons not having focus style (LemmyNet#1789)
  Add missing modlog reasons (LemmyNet#1787)
  Fix search page breaking on initial load when logged in (LemmyNet#1781)
  feat: Add PR template (LemmyNet#1785)
  v0.18.1-rc.8
  Fix profile loading spinner
  fix: Break text on post titles so long words don't overflow
  fix: Move getRoleLabelPill to the only component that uses it
  fix: Remove unused hasBadges() function
  fix: Fix badge alignment and break out into component
  fix: Fix up filter row gaps and margins a little
  fix: Fix heading levels
  ...
@jsit
Copy link
Contributor

jsit commented Jul 8, 2023

@SleeplessOne1917 I wanted to point out that it looks like Bootstrap does consider “secondary” to specifically be a light version of “body”:

IMG_1030

https://getbootstrap.com/docs/5.3/customize/color/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dessalines dessalines dessalines approved these changes

@alectrocute alectrocute Awaiting requested review from alectrocute

@jsit jsit Awaiting requested review from jsit

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@SleeplessOne1917 @jsit @dessalines