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

Add new dark theme: Dark berry blue #2065

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add new dark theme: Dark berry blue #2065

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f4249ce
Add new darktheme: Dark berry blue
ppenguin Aug 23, 2024
d19e3b4
theme ID mod, add description to improvements
ppenguin Aug 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ci/release/changelogs/next.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
- Sequence diagram: The spacing between self-referential edges and regular edges is uniform [#2043](https://github.com/terrastruct/d2/pull/2043)
- Compiler: Error on multi-line labels in `sql_table` shapes [#2057](https://github.com/terrastruct/d2/pull/2057)
- Sequence diagram: Image shape actors can use spans and notes [#2056](https://github.com/terrastruct/d2/issues/2056)
- Themes [#2065](https://github.com/terrastruct/d2/pull/2065):
- new theme `Dark Berry Blue` (`205`), intended as dark variant of `Mixed Berry Blue` (`5`).
- changed id of `Dark Flagship Terrastruct` (`203`) to mirror the ID of `Flagship Terrastruct` (`3`), improving theme discoverability.

#### Bugfixes ⛑️

Expand Down
1 change: 1 addition & 0 deletions d2themes/d2themescatalog/catalog.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ var LightCatalog = []d2themes.Theme{
var DarkCatalog = []d2themes.Theme{
DarkMauve,
DarkFlagshipTerrastruct,
DarkBerryBlue,
}

func Find(id int64) d2themes.Theme {
Expand Down
25 changes: 25 additions & 0 deletions d2themes/d2themescatalog/dark_berry_blue.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package d2themescatalog

import "oss.terrastruct.com/d2/d2themes"

var DarkBerryBlue = d2themes.Theme{
ID: 205,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense for mirroring themes but I think it may be a confusing precedent now that we agree the themes shouldn't be mirrored. Can you change this to just the next sequential dark theme ID? (202)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO "agreeing not to mirror all themes" is quite different from "mirroring some themes", and that the DX/UX of using this numbering scheme for themes that are mirrored is arguably better (the fact that category number groups are used does already mean that the numbers are not contiguous, so sequential numbering is a moot point). Instead I'd suggest to change the ID of the "flagship" theme to 203 and the world will be in order again 😉

(Note that it might even have advantages down the road for people who might want to programatically set their own "corporate identity" theme automatically to a user's preference).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I was wrong in my original comment about why we shouldn't mirror IDs. There shouldn't be any mirroring at all. D2's theme IDs are blocks of continuous ranges, where each range is a category of themes.

Starting from 0 they are cool.
Starting from 100 they are warm.
Starting from 200 they are dark.

Since it's not binary, mirroring doesn't even make sense. It's very possible 300s start up as something else entirely, like "3d", or "minimal".

In other words, it's not broken up like x = light and 20x = dark where you can make a case for mirroring.

Sequential numbering is not moot just because it's discontinuous between groups.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see why we couldn't have both, it's a perceived UX bonus at no extra cost (since "faux contiguous range" has no added value at all). But let's agree to disagree, not really worth haggeling over...

Name: "Dark berry blue",
Colors: d2themes.ColorPalette{
Neutrals: d2themes.DarkNeutral,

B1: "#E5F3FF",
B2: "#BCDDFB",
B3: "#77AFE3",
B4: "#3363DD",
B5: "#1F46B7",
B6: "#203586",

AA2: "#DACEFB",
AA4: "#A169D3",
AA5: "#6649B5",

AB4: "#9F7ED1",
AB5: "#BA69A6",
},
}
2 changes: 1 addition & 1 deletion d2themes/d2themescatalog/dark_flagship_terrastruct.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package d2themescatalog
import "oss.terrastruct.com/d2/d2themes"

var DarkFlagshipTerrastruct = d2themes.Theme{
ID: 201,
ID: 203,
Name: "Dark Flagship Terrastruct",
Colors: d2themes.ColorPalette{
Neutrals: d2themes.DarkNeutral,
Expand Down
Loading