www
/
www-new
12
3
Fork 5
Code Issues 58 Pull Requests 42 Projects Releases Wiki Activity

Implement dark theme (Closes #287) #407

Merged
e26chiu merged 13 commits from dark-theme into main 2022-06-08 08:45:30 -04:00
Conversation 13 Commits 13 Files Changed 37 +1675 -2046
e26chiu commented 2022-03-02 22:54:55 -05:00
Contributor
Copy Link
  • The website will have a dark theme or a light theme depending on your OS preferences. You can still customize in the themer page to override some properties.
  • Added a "Reset to dark mode" button in the themer page.

Staging link: https://csclub.uwaterloo.ca/~a3thakra/csc/dark-theme/

* The website will have a dark theme or a light theme depending on your OS preferences. You can still customize in the `themer` page to override some properties. * Added a "Reset to dark mode" button in the `themer` page. Staging link: https://csclub.uwaterloo.ca/~a3thakra/csc/dark-theme/
e26chiu added 2 commits 2022-03-02 22:54:56 -05:00
continuous-integration/drone/push Build is passing Details
e395d37d28 implement dark theme
continuous-integration/drone/push Build is passing Details
5e25bd2db1 Merge remote-tracking branch 'origin/main' into dark-theme
e26chiu requested review from n3parikh 2022-03-02 22:55:09 -05:00
e26chiu requested review from a258wang 2022-03-02 22:55:09 -05:00
e26chiu added 1 commit 2022-03-04 20:50:27 -05:00
continuous-integration/drone/push Build is passing Details
b26e587bb2 add toggle button & css media prefers-color-scheme
a3thakra reviewed 2022-03-05 15:16:25 -05:00
components/Footer.tsx Outdated
@ -18,0 +22,4 @@
<Button
size="small"
onClick={() =>
themeContext?.theme.name == "dark"
a3thakra commented 2022-03-05 15:16:25 -05:00
Owner
Copy Link

use ===

use ===
a3thakra marked this conversation as resolved
a3thakra reviewed 2022-03-05 15:25:39 -05:00
components/MiniEventCard.module.css Outdated
@ -67,0 +85,4 @@
.card:nth-child(odd) h2,
.card:nth-child(odd) h3,
.card:nth-child(odd) h4 {
color: var(--primary-subtitle);
a3thakra commented 2022-03-05 15:25:39 -05:00
Owner
Copy Link

two things:

  • this looks a little weird on the website - all text colors are exactly the same which seems like a design error
  • components should only care about their own design. With these rules, we are making them care about their neighbors was well. You should either do this in react explicitly or add it to the page's stylesheet instead of the component's stylesheet.
two things: - this looks a little weird on the website - all text colors are exactly the same which seems like a design error - components should only care about their own design. With these rules, we are making them care about their neighbors was well. You should either do this in react explicitly or add it to the page's stylesheet instead of the component's stylesheet.
e26chiu commented 2022-03-09 23:58:34 -05:00
Author
Contributor
Copy Link

What do you mean by doing this in react explicitely? Also, I tried to put this part in the parent's stylesheet but this seems not to affect the styling of the header (since .card is out of scope). The other solution I could think of is to pass the styling to the component from the page. Then again, I think it would be simpler to have the styling inside of the component instead.

Also, does that mean that the code with

.card:nth-child(odd) {
  background-color: var(--sectioning-light);
  color: var(--primary-subtitle);
}

will also need to be put outside of the component since it cares about its neighbors?

What do you mean by doing this in react explicitely? Also, I tried to put this part in the parent's stylesheet but this seems not to affect the styling of the header (since `.card` is out of scope). The other solution I could think of is to pass the styling to the component from the page. Then again, I think it would be simpler to have the styling inside of the component instead. Also, does that mean that the code with ```css .card:nth-child(odd) { background-color: var(--sectioning-light); color: var(--primary-subtitle); } ``` will also need to be put outside of the component since it cares about its neighbors?
a3thakra commented 2022-03-10 02:17:00 -05:00
Owner
Copy Link

Also, does that mean that the code with

.card:nth-child(odd) {
background-color: var(--sectioning-light);
color: var(--primary-subtitle);
}

will also need to be put outside of the component since it cares about its neighbors?

Yes

What do you mean by doing this in react explicitely? Also, I tried to put this part in the parent's stylesheet but this seems not to affect the styling of the header (since .card is out of scope). The other solution I could think of is to pass the styling to the component from the page. Then again, I think it would be simpler to have the styling inside of the component instead.

Yes, passing the prop is the way to go here. It is a little more work, but it makes a lot more sense in terms of maintainability. Something like this in the page component.

<MiniEventCard {...whatever other props} background={i % 2 === 0 ? "transparent" : "opaque"} />

and then you can apply a class on it based on whatever prop is passed in. That way your styles are still contained in the same css file, but the logic that dictates what an adjacent MiniEventCard looks like is not being leaked into the component styles.

> Also, does that mean that the code with > .card:nth-child(odd) { > background-color: var(--sectioning-light); > color: var(--primary-subtitle); > } > will also need to be put outside of the component since it cares about its neighbors? Yes > What do you mean by doing this in react explicitely? Also, I tried to put this part in the parent's stylesheet but this seems not to affect the styling of the header (since .card is out of scope). The other solution I could think of is to pass the styling to the component from the page. Then again, I think it would be simpler to have the styling inside of the component instead. Yes, passing the prop is the way to go here. It is a little more work, but it makes a lot more sense in terms of maintainability. Something like this in the [page component](https://git.csclub.uwaterloo.ca/www/www-new/src/branch/main/pages/events/%5Byear%5D/%5Bterm%5D/index.tsx#L104). ```ts <MiniEventCard {...whatever other props} background={i % 2 === 0 ? "transparent" : "opaque"} /> ``` and then you can apply a class on it based on whatever prop is passed in. That way your styles are still contained in the same css file, but the logic that dictates what an adjacent MiniEventCard looks like is not being leaked into the component styles.
a3thakra commented 2022-03-10 02:17:36 -05:00
Owner
Copy Link

Note that this is not as important as the other issues that I have mentioned.

Note that this is not as important as the other issues that I have mentioned.
a3thakra marked this conversation as resolved
a3thakra reviewed 2022-03-05 15:27:29 -05:00
components/MiniEventCard.module.css Outdated
@ -34,2 +40,4 @@
}
.card:nth-child(odd) .details {
color: var(--primary-subtitle);
a3thakra commented 2022-03-05 15:27:29 -05:00
Owner
Copy Link

view details is a link, and it should be visible as a link. we shouldn't change this color

view details is a link, and it should be visible as a link. we shouldn't change this color
e26chiu commented 2022-03-06 07:50:50 -05:00
Author
Contributor
Copy Link

This colour is in the Figma design: https://www.figma.com/file/vu3AIDmN8vLALfwPbtPhO4/CS-Club-Official-Figma?node-id=639%3A484 Should we change it just for the sake of keeping it as a link?

This colour is in the Figma design: https://www.figma.com/file/vu3AIDmN8vLALfwPbtPhO4/CS-Club-Official-Figma?node-id=639%3A484 Should we change it just for the sake of keeping it as a link?
a3thakra commented 2022-03-10 02:10:11 -05:00
Owner
Copy Link

Although figma is our source of truth and we should stick to it as much as possible, bit it is not set in stone. We can give this feedback to the design team and raise the issue with them.

IMO if we don't make it look like something will happen if we click on it - then it is not drawing the user's attention to click on it - and they probably wont - which is not good.

Although figma is our source of truth and we should stick to it as much as possible, bit it is not set in stone. We can give this feedback to the design team and raise the issue with them. IMO if we don't make it look like something will happen if we click on it - then it is not drawing the user's attention to click on it - and they probably wont - which is not good.
a3thakra marked this conversation as resolved
a3thakra commented 2022-03-05 15:35:37 -05:00
Owner
Copy Link

mobile navbar in dark mode has some problems

mobile navbar in dark mode has some problems
Screenshot_20220305-123522_Brave.jpg
229 KiB
Screenshot_20220305-123524_Brave.jpg
383 KiB
Screenshot_20220305-123522_Brave.jpg Screenshot_20220305-123524_Brave.jpg
🚀 1
a3thakra commented 2022-03-05 15:41:38 -05:00
Owner
Copy Link

table rows look a little intense compared to light theme

table rows look a little intense compared to light theme
Screenshot_20220305-124009_Brave.jpg
398 KiB
Screenshot_20220305-124107_Brave.jpg
430 KiB
Screenshot_20220305-124009_Brave.jpg Screenshot_20220305-124107_Brave.jpg
🚀 1
a3thakra commented 2022-03-05 15:43:04 -05:00
Owner
Copy Link

mobile organized content is unthemed

mobile organized content is unthemed
Screenshot_20220305-124206_Brave.jpg
298 KiB
Screenshot_20220305-124206_Brave.jpg
🚀 1
a3thakra commented 2022-03-05 15:49:41 -05:00
Owner
Copy Link

good job overall! 🎉

i think more people should take passes on this. perhaps you should share the staging link with the design team and ask them to take a look.

other internal members could also help with reviewing once you announce this at all hands. make sure to share a link to the staging website.

let everyone know that they can easily toggle between light and dark using the button in the footer.

good job overall! 🎉 i think more people should take passes on this. perhaps you should share the staging link with the design team and ask them to take a look. other internal members could also help with reviewing once you announce this at all hands. make sure to share a link to the staging website. let everyone know that they can easily toggle between light and dark using the button in the footer.
e26chiu added 2 commits 2022-04-03 23:24:40 -04:00
continuous-integration/drone/push Build is failing Details
feb04132fe refactored light/dark mode
a3thakra reviewed 2022-04-21 22:14:55 -04:00
components/Bubble.module.css Outdated
@ -6,3 +6,3 @@
.bubble {
--border-radius: calc(5000rem / 16);
border-radius: calc(5000rem / 16);
a3thakra commented 2022-04-21 22:14:55 -04:00
Owner
Copy Link
  --border-radius: calc(5000rem / 16);
```suggestion --border-radius: calc(5000rem / 16); ```
a3thakra commented 2022-06-03 04:16:59 -04:00
Owner
Copy Link

@e26chiu the bubbles are still broken :'(

Go to the about us page to see the difference: https://csclub.uwaterloo.ca/~a3thakra/csc/dark-theme/about/

Once you change this back to a variable as I suggested above, the bubbles should be fixed.

@e26chiu the bubbles are still broken :'( Go to the about us page to see the difference: https://csclub.uwaterloo.ca/~a3thakra/csc/dark-theme/about/ Once you change this back to a variable as I suggested above, the bubbles should be fixed.
a3thakra commented 2022-04-21 22:21:11 -04:00
Owner
Copy Link
  • talk to the design team to figure out bright blue color
  • Font size of the heading is not consistent
<img src="/attachments/bcf08984-951d-4528-8a62-08182d08c70e" height="400px" /> - talk to the design team to figure out bright blue color - Font size of the heading is not consistent
image.png
404 KiB
image.png
404 KiB
image.png
🚀 1
a3thakra reviewed 2022-04-21 22:24:11 -04:00
components/MiniEventCard.tsx Outdated
@ -11,6 +11,7 @@ interface Props {
online: boolean;
location: string;
date: Date;
background: string;
a3thakra commented 2022-04-21 22:24:11 -04:00
Owner
Copy Link

background: "light" | "dark"

`background: "light" | "dark"`
a3thakra commented 2022-04-21 22:27:41 -04:00
Owner
Copy Link
  • "View details" color
  • wrong background color
<img src="/attachments/51b82f68-19db-4dec-ab5e-ba4bc071fb22" height="400px" /> - "View details" color - wrong background color
image.png
619 KiB
🚀 1
a3thakra commented 2022-04-21 22:29:25 -04:00
Owner
Copy Link

Non blocker for PR: Create another issue for bad scrollbar color image

Non blocker for PR: Create another issue for bad scrollbar color ![image](/attachments/37f434d9-71e9-43d3-934f-30ba9f822ba5)
image.png
63 KiB
a3thakra commented 2022-04-21 22:30:42 -04:00
Owner
Copy Link

Chat with the design team to figure out a better color for this because this stands out too much

image

Chat with the design team to figure out a better color for this because this stands out too much ![image](/attachments/268c57c5-0902-4fe2-973f-1b2f1269cc3c)
image.png
65 KiB
a3thakra reviewed 2022-04-21 22:33:40 -04:00
pages/_app.css Outdated
@ -6,3 +6,2 @@
/* Default is light theme */
--primary-background: #ffffff;
--secondary-background: #fdf8f5;
--light-primary-background: #ffffff;
a3thakra commented 2022-04-21 22:33:40 -04:00
Owner
Copy Link

--light--primary-background and --dark--primary-background

`--light--primary-background` and `--dark--primary-background`
a3thakra reviewed 2022-04-21 22:37:50 -04:00
components/Theme.tsx Outdated
@ -108,3 +133,3 @@
if (customPalette == null) {
setThemeName("light");
const prefersDark = window.matchMedia(
a3thakra commented 2022-04-21 22:37:50 -04:00
Owner
Copy Link

I don't think we need this, we already have this in css.

I don't think we need this, we already have this in css.
a3thakra reviewed 2022-04-21 22:39:41 -04:00
components/Theme.tsx Outdated
@ -85,2 +108,4 @@
document.body.classList.add("light");
} else if (input === "dark") {
document.body.classList.add("dark");
document.body.classList.remove("light");
a3thakra commented 2022-04-21 22:39:41 -04:00
Owner
Copy Link

We don't need light and dark classes anymore (not in css either). We can use PALETTE_NAMES.forEach(...) to programatically set the prefix on the theme.

We don't need `light` and `dark` classes anymore (not in css either). We can use `PALETTE_NAMES.forEach(...)` to programatically set the prefix on the theme.
e26chiu added 3 commits 2022-05-05 22:39:55 -04:00
continuous-integration/drone/push Build is failing Details
70c657ec4b update typescript
e26chiu added 1 commit 2022-05-05 23:04:50 -04:00
continuous-integration/drone/push Build is passing Details
caa44e6316 fix lint errors
e26chiu added 1 commit 2022-05-22 08:28:35 -04:00
continuous-integration/drone/push Build is passing Details
3a4581a8b9 fix sidebar & mini event card colour + navbar overlay
a258wang requested changes 2022-05-30 01:22:56 -04:00
a258wang left a comment
Owner
Copy Link

Wow! This is big, great work Emily! There just a few things I noticed while going through the staging site that we should address before shipping this:

  1. Bubble component: Border radius (see Adi's comment)
  2. Table component (Members page, Services page > SSH Key Fingerprints): In dark mode, the vertical lines are same colour as header background, maybe we can consider changing that?
  3. Table component: In light mode, the header is now bright green instead of the nice pale green we currently have on the website, could we please keep the pale green? (Adi also commented on this)
  4. Events page: "View details" arrow on odd-numbered cards (secondary colour cards) is not flipping (Oh wait Adi commented on this as well)
  5. Events page: In light mode, all cards are white instead of alternating green/white (Same Adi comment as above)
  6. Mobile, Events page: In both colour themes, the spacing above "View details" on odd-numbered (secondary colour) cards looks off? (maybe this will be fixed by merging in main, idk)

I also left some nitpicky comments about code style, those are less important than these user-facing visual bugs though. 🙂

Wow! This is big, great work Emily! There just a few things I noticed while going through the staging site that we should address before shipping this: 1. Bubble component: Border radius (see [Adi's comment](https://git.csclub.uwaterloo.ca/www/www-new/pulls/407/files#issuecomment-9445)) 2. Table component (Members page, Services page > SSH Key Fingerprints): In dark mode, the vertical lines are same colour as header background, maybe we can consider changing that? 3. Table component: In light mode, the header is now bright green instead of the nice pale green we currently have on the website, could we please keep the pale green? ([Adi also commented on this](https://git.csclub.uwaterloo.ca/www/www-new/pulls/407#issuecomment-9179)) 4. Events page: "View details" arrow on odd-numbered cards (secondary colour cards) is not flipping ([Oh wait Adi commented on this as well](https://git.csclub.uwaterloo.ca/www/www-new/pulls/407#issuecomment-9451)) 5. Events page: In light mode, all cards are white instead of alternating green/white ([Same Adi comment as above](https://git.csclub.uwaterloo.ca/www/www-new/pulls/407#issuecomment-9451)) 6. Mobile, Events page: In both colour themes, the spacing above "View details" on odd-numbered (secondary colour) cards looks off? (maybe this will be fixed by merging in main, idk) I also left some nitpicky comments about code style, those are less important than these user-facing visual bugs though. 🙂
table-head-dark.png
27 KiB
table-head-light.png
24 KiB
events-arrow.png
129 KiB
events-background-colour.png
85 KiB
mobile-events-spacing.png
37 KiB
table-head-dark.png table-head-light.png events-arrow.png events-background-colour.png mobile-events-spacing.png
components/MiniEventCard.module.css Outdated
@ -9,0 +9,4 @@
box-sizing: border-box;
position: relative;
padding: calc(20rem / 16);
background-color: var(--card-background);
a258wang commented 2022-05-30 00:59:13 -04:00
Owner
Copy Link

Idea: since there are some repeated styles between .card and .darkBg, what if we "had all the default style" apply to .card, and then use a selector like .card.darkBg as an "override" for the background colour? (or even just .darkBg should work, as long as it comes after the other selector)
eg.

.card {
  box-sizing: ...
  position: ...
  padding: ...
  color: ...
  background-color: ...
}

.card.darkBg { /* or even just .darkBg */
  background-color: ... /* like an override */
}

In order for this to work though, we would have to slightly change how the classNames are applied.

Idea: since there are some repeated styles between `.card` and `.darkBg`, what if we "had all the default style" apply to `.card`, and then use a selector like `.card.darkBg` as an "override" for the background colour? (or even just `.darkBg` should work, as long as it comes after the other selector) eg. ```css .card { box-sizing: ... position: ... padding: ... color: ... background-color: ... } .card.darkBg { /* or even just .darkBg */ background-color: ... /* like an override */ } ``` In order for this to work though, we would have to slightly change how the classNames are applied.
👍 1
components/MiniEventCard.module.css Outdated
@ -11,3 +15,3 @@
.name {
display: flex;
font-size: calc(18rem / 16);
font-size: 18px;
a258wang commented 2022-05-30 00:52:32 -04:00
Owner
Copy Link

Why are we using px instead of rem here?

Why are we using `px` instead of `rem` here?
components/MiniEventCard.module.css Outdated
@ -49,25 +53,39 @@
list-style: none;
}
.darkBg > summary {
a258wang commented 2022-05-30 00:31:39 -04:00
Owner
Copy Link

NIT: For this and the other styles below, it would be shorter to write

.card > summary,
.darkBg > summary {
  ...
}
NIT: For this and the other styles below, it would be shorter to write ```css .card > summary, .darkBg > summary { ... } ```
components/Navbar.tsx Outdated
@ -121,3 +121,3 @@
onClick={() => dispatch({ type: "open", route: router.pathname })}
>
<Image src="/images/hamburger.svg" alt="Menu" />
<svg
a258wang commented 2022-05-30 00:37:07 -04:00
Owner
Copy Link

NIT: Instead of writing out these SVGs inline, it might be more readable to extract them into components (functions) like HamburgerSvg and DropdownSVG? (but keeping them in the same file, just at the bottom)

See the SocialLinks component for an illustrative example.

NIT: Instead of writing out these SVGs inline, it might be more readable to extract them into components (functions) like `HamburgerSvg` and `DropdownSVG`? (but keeping them in the same file, just at the bottom) See the [SocialLinks component](https://git.csclub.uwaterloo.ca/www/www-new/src/branch/main/components/SocialLinks.tsx#L70) for an illustrative example.
pages/events/[year]/[term]/index.tsx Outdated
@ -97,3 +97,3 @@
)}
<div className={styles.miniEventCards}>
{props.pastEvents.map(({ content, metadata }) => (
{props.pastEvents.map((event, key) => (
a258wang commented 2022-05-30 01:00:45 -04:00
Owner
Copy Link

Super NIT: rename key to idx 😛

Super NIT: rename `key` to `idx` 😛
pages/resources/advice/co-op.module.css
@ -31,2 +32,4 @@
}
.adviceBarContainer a:hover {
color: var(--link-hover);
a258wang commented 2022-05-30 01:02:02 -04:00
Owner
Copy Link

EDIT: This relates to #426, but I think the actual fix is slightly different/beyond the scope of the dark theme

EDIT: This relates to #426, but I think the actual fix is slightly different/beyond the scope of the dark theme
e26chiu added 1 commit 2022-05-30 23:28:10 -04:00
continuous-integration/drone/push Build is passing Details
6dde3f7614 fix events + table + svg
e26chiu commented 2022-05-30 23:30:00 -04:00
Author
Contributor
Copy Link

The most recent push should fix all of @a258wang 's comments and suggestions! Note: I believe point #1 (border-radius) was already resolved.

The most recent push should fix all of @a258wang 's comments and suggestions! Note: I believe point #1 (border-radius) was already resolved.
a3thakra commented 2022-06-03 04:21:07 -04:00
Owner
Copy Link

News archive just looks like a wall of text compared to the light theme.
It might need brighter headlines.

https://csclub.uwaterloo.ca/~a3thakra/csc/dark-theme/news/2022/spring/

image

News archive just looks like a wall of text compared to the light theme. It might need brighter headlines. https://csclub.uwaterloo.ca/~a3thakra/csc/dark-theme/news/2022/spring/ ![image](/attachments/28cf74bb-61c3-4cd3-aad1-18bf20221979)
image.png
369 KiB
a3thakra reviewed 2022-06-03 04:30:55 -04:00
a258wang reviewed 2022-06-04 13:38:38 -04:00
components/Table.module.css
@ -17,3 +17,2 @@
.table tbody tr:nth-child(odd) {
background: var(--primary-accent-lightest);
.table tbody tr:nth-child(even) {
a258wang commented 2022-06-04 13:38:38 -04:00
Owner
Copy Link

Just curious, why was this changed from odd to even?

Just curious, why was this changed from `odd` to `even`?
e26chiu commented 2022-06-06 08:08:23 -04:00
Author
Contributor
Copy Link

I think I misread what you wrote:

Table component (Members page, Services page > SSH Key Fingerprints): In dark mode, the vertical lines are same colour as header background, maybe we can consider changing that?

I thought the background of the first row needed to be changed because in dark mode, it's very similar to the header (might not contrast well enough). However, you were talking about the colours of the vertical lines. 😅

I think I misread what you wrote: > Table component (Members page, Services page > SSH Key Fingerprints): In dark mode, the vertical lines are same colour as header background, maybe we can consider changing that? I thought the background of the first row needed to be changed because in dark mode, it's very similar to the header (might not contrast well enough). However, you were talking about the colours of the vertical lines. 😅
😆 1
e26chiu commented 2022-06-06 08:52:27 -04:00
Author
Contributor
Copy Link

For the vertical lines in the table, do you think this would be better?
Table with lighter vertical lines
I don't think there is an issue with having a similar colour for the vertical lines and the header as long as there is enough spacing between each header headings and column text. However, it would make sense to change the colour of the vertical lines if we want to keep the same design for the light theme and the dark theme (the light theme has a vertical line colour different than the header's colour).

For the vertical lines in the table, do you think this would be better? ![Table with lighter vertical lines](https://imgur.com/DSfoGdg.png) I don't think there is an issue with having a similar colour for the vertical lines and the header as long as there is enough spacing between each header headings and column text. However, it would make sense to change the colour of the vertical lines if we want to keep the same design for the light theme and the dark theme (the light theme has a vertical line colour different than the header's colour).
a258wang commented 2022-06-08 00:34:31 -04:00
Owner
Copy Link

I think what you have in this screenshot looks fine! It would be nice to have a similar design for both light + dark themes, but I agree that it's not a big deal in this particular case - let's not block this PR over it, and we can always make a ticket to revisit it later. 🙂

I think what you have in this screenshot looks fine! It would be nice to have a similar design for both light + dark themes, but I agree that it's not a big deal in this particular case - let's not block this PR over it, and we can always make a ticket to revisit it later. 🙂
a258wang reviewed 2022-06-04 13:43:14 -04:00
pages/_app.css Outdated
@ -35,0 +47,4 @@
--light--button-background: #1482e3;
--light--footer-background: #2a2a62;
--light--card-background: #DCF6F0;
a258wang commented 2022-06-04 13:43:14 -04:00
Owner
Copy Link

Careful here, now the news cards on the home page are green 😂 https://csclub.uwaterloo.ca/~a3thakra/csc/dark-theme/

We can either change around the usage of the existing variables, or create a new variable if necessary.

Careful here, now the news cards on the home page are green 😂 https://csclub.uwaterloo.ca/~a3thakra/csc/dark-theme/ We can either change around the usage of the existing variables, or create a new variable if necessary.
👍 1
e26chiu commented 2022-06-06 08:16:36 -04:00
Author
Contributor
Copy Link

News archive just looks like a wall of text compared to the light theme.
It might need brighter headlines.

https://csclub.uwaterloo.ca/~a3thakra/csc/dark-theme/news/2022/spring/

image

@a3thakra Do you think adding space between each news item would make the page look less like a wall of text? I think it would make each news item more distinguishable since currently, the spacing is not very consistent: there is more spacing between the paragraphs of the news item than the one between the news items themselves.
It would also follow the Gestalt proximity principle of linking items together when they are closer to each other.

> News archive just looks like a wall of text compared to the light theme. > It might need brighter headlines. > > https://csclub.uwaterloo.ca/~a3thakra/csc/dark-theme/news/2022/spring/ > > ![image](/attachments/28cf74bb-61c3-4cd3-aad1-18bf20221979) @a3thakra Do you think adding space between each news item would make the page look less like a wall of text? I think it would make each news item more distinguishable since currently, the spacing is not very consistent: there is more spacing between the paragraphs of the news item than the one between the news items themselves. It would also follow the [Gestalt proximity principle](https://maze.co/blog/gestalt-grouping-principles/) of linking items together when they are closer to each other.
e26chiu added 1 commit 2022-06-06 23:13:08 -04:00
continuous-integration/drone/push Build is passing Details
bf1c829bf6 fix bubble + colour on events page
e26chiu added 1 commit 2022-06-07 20:12:58 -04:00
continuous-integration/drone/push Build is passing Details
4ec206046f merge from main
a258wang commented 2022-06-08 00:36:20 -04:00
Owner
Copy Link

Do you think adding space between each news item would make the page look less like a wall of text? I think it would make each news item more distinguishable since currently, the spacing is not very consistent: there is more spacing between the paragraphs of the news item than the one between the news items themselves.
It would also follow the Gestalt proximity principle of linking items together when they are closer to each other.

+1, I think the news items on the news archive page need more space between each other (regardless of light vs. dark mode). We can make a ticket to take a closer look at this later.

> Do you think adding space between each news item would make the page look less like a wall of text? I think it would make each news item more distinguishable since currently, the spacing is not very consistent: there is more spacing between the paragraphs of the news item than the one between the news items themselves. > It would also follow the [Gestalt proximity principle](https://maze.co/blog/gestalt-grouping-principles/) of linking items together when they are closer to each other. +1, I think the news items on the news archive page need more space between each other (regardless of light vs. dark mode). We can make a ticket to take a closer look at this later.
a258wang approved these changes 2022-06-08 00:40:14 -04:00
a258wang left a comment
Owner
Copy Link

Thanks for working on this @e26chiu! I'd say let's get this PR merged sooner rather than later, then we can address the remaining details and any small bugs that arise in follow-up tickets. 🚢

Thanks for working on this @e26chiu! I'd say let's get this PR merged sooner rather than later, then we can address the remaining details and any small bugs that arise in follow-up tickets. 🚢
e26chiu merged commit f631f4013f into main 2022-06-08 08:45:30 -04:00
a3thakra commented 2022-06-18 06:03:19 -04:00
Owner
Copy Link

@a3thakra Do you think adding space between each news item would make the page look less like a wall of text? I think it would make each news item more distinguishable since currently, the spacing is not very consistent: there is more spacing between the paragraphs of the news item than the one between the news items themselves.
It would also follow the Gestalt proximity principle of linking items together when they are closer to each other.

Yes I think that should do it! :)

Sorry for the late response, I haven't been checking mattermost or my csc emails too often in the past couple weeks :'(

> @a3thakra Do you think adding space between each news item would make the page look less like a wall of text? I think it would make each news item more distinguishable since currently, the spacing is not very consistent: there is more spacing between the paragraphs of the news item than the one between the news items themselves. > It would also follow the [Gestalt proximity principle](https://maze.co/blog/gestalt-grouping-principles/) of linking items together when they are closer to each other. Yes I think that should do it! :) Sorry for the late response, I haven't been checking mattermost or my csc emails too often in the past couple weeks :'(
a3thakra deleted branch dark-theme 2022-06-18 06:03:24 -04:00
Sign in to join this conversation.
Reviewers
No reviewers
n3parikh
a258wang
Labels
Clear labels   
a11y

   
Backlog

   
Blocked

   
Bug

   
Content

   
Dependencies

   
Design

   
Feature Request

   
Good First Issue

   
In Progress

   
Performance

   
Priority - High

   
Priority - Low

   
Priority - Medium

   
Untriaged
No Label
a11y
Backlog
Blocked
Bug
Content
Dependencies
Design
Feature Request
Good First Issue
In Progress
Performance
Priority - High
Priority - Low
Priority - Medium
Untriaged
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: www/www-new#407
No description provided.
Delete Branch "dark-theme"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?