infrastructure/conference-website
1
1
Fork 1
Code Issues 14 Pull Requests Releases Activity

UI: Implement Web Assets' theme system, and add 'dark' theme #103972

Merged
Márton Lente merged 23 commits from ui/theme-system into main 2024-09-30 15:03:39 +02:00
Conversation 15 Commits 23 Files Changed 13 +116 -18
Márton Lente commented 2024-09-25 17:10:50 +02:00
Owner
Copy Link

This pull request enables the theme system, and the optional Web Assets v2 dark theme. It resolves local contrast issues, and makes the necessary project-specific changes. It addresses dynamic location colours' theme-specific adjustments as well.

The theme remains light by default, that users can set to either 'system-preferred' or 'light' respectively. User selection is stored in browser's local storage, and will prevail on next page loads.

Screenshots:

image

image

image

This pull request enables the theme system, and the optional Web Assets v2 dark theme. It resolves local contrast issues, and makes the necessary project-specific changes. It addresses dynamic location colours' theme-specific adjustments as well. The theme remains light by default, that users can set to either 'system-preferred' or 'light' respectively. User selection is stored in browser's local storage, and will prevail on next page loads. **Screenshots:** ![image](https://projects.blender.org/attachments/6969cbac-2f1c-461d-bf3e-10df63b38d45) ![image](https://projects.blender.org/attachments/df341e56-897a-4aca-bb0b-631b2024ff18) ![image](https://projects.blender.org/attachments/6cb30585-d5ed-4f0f-b0f8-6e6fe9a3c986)
ui-implement-web-assets-theme-system-and-add-dark-theme.png
1010 KiB
ui-implement-web-assets-theme-system-and-add-dark-theme-2.png
2.1 MiB
ui-implement-web-assets-theme-system-and-add-dark-theme-1.png
190 KiB
Márton Lente added 7 commits 2024-09-25 17:10:53 +02:00 
Initialize Web Assets v2 theme systems scripts and styles, and set theme
defaults.
Change static colours to dynamic colour variables to enable the theme system
base.
Make component event box colours work with dark and light themes, by removing
explicit colour settings and adding backdrop-filter rules.
Add schedule style mixins to introduce dynamic contrast for dark and light
themes by adjusting location colour luminance values based on active theme.
Márton Lente added 1 commit 2024-09-26 11:08:01 +02:00 
Add list-group-item background-colour  style override for theme system
compatibility.
Márton Lente added 1 commit 2024-09-26 11:09:15 +02:00
Márton Lente added 1 commit 2024-09-26 11:14:01 +02:00
Márton Lente changed title from WIP: Implement Web Assets' theme system, and add 'dark' theme to UI: Implement Web Assets' theme system, and add 'dark' theme 2024-09-26 12:05:19 +02:00
Márton Lente requested review from Pablo Vazquez 2024-09-26 12:05:27 +02:00
Pablo Vazquez requested changes 2024-09-26 12:26:54 +02:00
conference_main/static/conference_main/styles/main.sass Outdated
@ -22,3 +24,3 @@
--color-bg: var(--body-color-bg)
--navbar-bg: #1e1e1e
// --box-bg-color-hover: var(--btn-color-bg-hover)
Pablo Vazquez commented 2024-09-26 12:25:50 +02:00
Owner
Copy Link

Left over comment

Left over comment
martonlente marked this conversation as resolved
conference_main/static/conference_main/styles/main.sass Outdated
@ -261,6 +270,10 @@ button.going-star
.form-check-input
transform: scale(1.5) translateX(calc(var(--spacer) * -1))
// TODO: remove rule after Web Assets v2 component nav-pills has been implemented
Pablo Vazquez commented 2024-09-26 12:26:13 +02:00
Owner
Copy Link

Hasn't this be implemented already? 08054cfd5f

Hasn't this be implemented already? https://projects.blender.org/infrastructure/web-assets/commit/08054cfd5f99c9097da9ec19ea4e5fe5156bd6e3
Márton Lente commented 2024-09-26 13:08:15 +02:00
Author
Owner
Copy Link

Thanks for the reference. It was added to web-assets, but not yet implemented to the Conference website at the time of the commit. Now it's present on main, and I cleaned up the style in the PR that became redundant in the meanwhile. 👍

Thanks for the reference. It was added to web-assets, but not yet implemented to the Conference website at the time of the commit. Now it's present on `main`, and I cleaned up the style in the PR that became redundant in the meanwhile. 👍
martonlente marked this conversation as resolved
Pablo Vazquez commented 2024-09-26 12:44:13 +02:00
Owner
Copy Link

Can you run this reliably? When I switch pages I get several glitches:

  • Sometimes the theme button is not clickable
  • Sometimes it switches back to a different setting

Testing this after deleting the /public/static folder (several times). Using Chrome, dev tools with caching disabled so it's fresh every load. (Local storage is persistent though).

Can you run this reliably? When I switch pages I get several glitches: * Sometimes the theme button is not clickable * Sometimes it switches back to a different setting Testing this after deleting the `/public/static` folder (several times). Using Chrome, dev tools with caching disabled so it's fresh every load. (Local storage is persistent though). <video src="/attachments/41152d4a-3b8d-46a6-bc49-e0e1f0d9d182" title="conf_theme_switch.mov" controls></video>
conf_theme_switch.mov
29 MiB
Pablo Vazquez commented 2024-09-26 12:52:07 +02:00
Owner
Copy Link

Managed to reproduce twice again. But can't figure out the steps.

This is how I'm testing it:

  • Delete /public/static folder.
  • Open DevTools, disable cache
  • Remove local storage keys (to start fresh)
  • Open http://conference.local:8009/ and start clicking back and forth between flatpages (Location), Speakers/Attendees, and Home.

My system appareance is set to Light. Using Chrome on macOS Sequoia.

Managed to reproduce twice again. But can't figure out the steps. This is how I'm testing it: * Delete `/public/static` folder. * Open DevTools, disable cache * Remove local storage keys (to start fresh) * Open http://conference.local:8009/ and start clicking back and forth between flatpages (Location), Speakers/Attendees, and Home. My system appareance is set to Light. Using Chrome on macOS Sequoia. <video src="/attachments/81c82de8-2dcc-49df-931e-03ec4a6a6973" title="conf_theme_switch_2.mov" controls></video>
conf_theme_switch_2.mov
17 MiB
👍 1
Márton Lente added 1 commit 2024-09-26 13:00:23 +02:00
Márton Lente added 1 commit 2024-09-26 13:02:28 +02:00 
Part of #103972
Márton Lente added 1 commit 2024-09-26 13:05:38 +02:00 
Remove redundant style override after component nav-pills has been implemented
on main.
Part of #103972
Márton Lente added 1 commit 2024-09-26 15:09:02 +02:00 
Fixes theme system JS and theme toggle button issues as reported in the pull
request description. See the PR for details.
Part of #103972
Márton Lente commented 2024-09-26 15:17:00 +02:00
Author
Owner
Copy Link

Managed to reproduce twice again. But can't figure out the steps.

This is how I'm testing it:

  • Delete /public/static folder.
  • Open DevTools, disable cache
  • Remove local storage keys (to start fresh)
  • Open http://conference.local:8009/ and start clicking back and forth between flatpages (Location), Speakers/Attendees, and Home.

My system appareance is set to Light. Using Chrome on macOS Sequoia.

@pablovazquez thanks for the detailed reports!

The above issues were resolved, and the fix has been released as Web Assets v2.0.0-alpha.41, as discussed IRL. I recorded the details in the following issue for reference: infrastructure/web-assets#94705 Theme features should now work consistently in the project in all browsers.

> Managed to reproduce twice again. But can't figure out the steps. > > This is how I'm testing it: > * Delete `/public/static` folder. > * Open DevTools, disable cache > * Remove local storage keys (to start fresh) > * Open http://conference.local:8009/ and start clicking back and forth between flatpages (Location), Speakers/Attendees, and Home. > > My system appareance is set to Light. Using Chrome on macOS Sequoia. > > <video src="/attachments/81c82de8-2dcc-49df-931e-03ec4a6a6973" title="conf_theme_switch_2.mov" controls></video> @pablovazquez thanks for the detailed reports! The above issues were resolved, and the fix has been released as Web Assets [v2.0.0-alpha.41](https://projects.blender.org/infrastructure/web-assets/releases/tag/v2.0.0-alpha.41), as discussed IRL. I recorded the details in the following issue for reference: https://projects.blender.org/infrastructure/web-assets/issues/94705 Theme features should now work consistently in the project in all browsers.
Márton Lente requested review from Pablo Vazquez 2024-09-26 15:18:15 +02:00
Pablo Vazquez commented 2024-09-26 15:49:30 +02:00
Owner
Copy Link

Did you change the location colors locally? With the current default color for the Theater (#5f1bef) I get really low contrast (1.06) on presentation details. Example

low contrast

Did you change the location colors locally? With the current default color for the Theater (`#5f1bef`) I get really low contrast (1.06) on presentation details. [Example](http://conference.local:8009/2019/presentations/563/) ![low contrast](/attachments/2b7e66d6-2abe-4c57-b093-d633b102c890)
image.png
98 KiB
Pablo Vazquez commented 2024-09-26 15:52:06 +02:00
Owner
Copy Link

The "socket" of unselected filters is still pure white

filters

The "socket" of unselected filters is still pure white ![filters](/attachments/15d6f59f-45fe-4449-bc62-1d4ccbd7ea10)
image.png
28 KiB
Pablo Vazquez commented 2024-09-26 15:54:04 +02:00
Owner
Copy Link

Unclaimed tickets copy field is unstyled

unclaimed tickets

Unclaimed tickets copy field is unstyled ![unclaimed tickets](/attachments/35de8828-8938-4149-bbb1-e00d6ede0689)
image.png
132 KiB
Pablo Vazquez commented 2024-09-26 16:22:01 +02:00
Owner
Copy Link

The horizontal schedule items are a bit too low contrast, they look almost "disabled".

Current

backdrop-filter: brightness(0.9);:
low contrast

Proposed

backdrop-filter: brightness(0.75);:
proposed

What if we styled the favorite/going items differently? Just a mockup:
favorite/going

The horizontal schedule items are a bit too low contrast, they look almost "disabled". #### Current `backdrop-filter: brightness(0.9);`: ![low contrast](/attachments/297d3547-c4e9-49e8-bbf0-ff08afe4b66d) ### Proposed `backdrop-filter: brightness(0.75);`: ![proposed](/attachments/3698f54b-6977-4533-8d8d-7c92ef5b5076) What if we styled the favorite/going items differently? Just a mockup: ![favorite/going](/attachments/2c6adf50-1fe7-4f76-858d-bb2a952baad9)
image.png
86 KiB
image.png
88 KiB
image.png
97 KiB
Márton Lente added 1 commit 2024-09-26 16:23:02 +02:00 
Part of #103972
Márton Lente added 1 commit 2024-09-26 16:31:32 +02:00 
Part of #103972
Pablo Vazquez commented 2024-09-26 16:38:26 +02:00
Owner
Copy Link

There is a flicker in loading the theme that is quite noticeable on pages heavy on images, like Speakers, Attendees, or Schedule.

This is not just a problem of our implementation but something many run into when introducing themes.

Some propose using a bit of inline Javascript (vanilla) in the <head>, before anything, which is probably the best. Outlined in this StackExchange answer or this GitHub comment.

There is a flicker in loading the theme that is quite noticeable on pages heavy on images, like Speakers, Attendees, or Schedule. <video src="/attachments/cd3c3ac2-d7e9-4ee0-b8df-d16324b99fcd" title="slow_switch.mov" controls></video> This is not just a problem of our implementation but something many run into when introducing themes. Some propose using a bit of inline Javascript (vanilla) in the `<head>`, before anything, which is probably the best. Outlined in [this StackExchange answer](https://stackoverflow.com/a/62635541/8444393) or [this GitHub comment](https://github.com/vercel/next.js/discussions/12533#discussioncomment-6696252).
slow_switch.mov
4.0 MiB
Márton Lente added 1 commit 2024-09-26 16:44:50 +02:00 
Part of #103972
Márton Lente added 1 commit 2024-09-26 16:51:51 +02:00 
Increase schedule event items' colour contrasts in theme dark, and make
background colours lighter on hover instead of darker to follow existing hover
behaviour better.
Part of #103972
Márton Lente added 1 commit 2024-09-26 19:01:02 +02:00 
Add styles to make favourited events colour contrast stronger in horizontal
schedule layout.
Part of #103972
Márton Lente commented 2024-09-26 19:12:38 +02:00
Author
Owner
Copy Link

Did you change the location colors locally? With the current default color for the Theater (#5f1bef) I get really low contrast (1.06) on presentation details. Example

I didn't change any location colours explicitly, but introduced dynamic luminance adjustments based on active theme. As location colours are applied on a per component basis, I overlooked some items. I did another pass based on the above, all items should look correct now. If we come accross any items that are displayed with low contrast, we can add more luminance values to theme-dark-schedule and theme-light-schedule mixins.

> Did you change the location colors locally? With the current default color for the Theater (`#5f1bef`) I get really low contrast (1.06) on presentation details. [Example](http://conference.local:8009/2019/presentations/563/) I didn't change any location colours explicitly, but introduced dynamic luminance adjustments based on active theme. As location colours are applied on a per component basis, I overlooked some items. I did another pass based on the above, all items should look correct now. If we come accross any items that are displayed with low contrast, we can add more luminance values to `theme-dark-schedule` and `theme-light-schedule` mixins.
Márton Lente commented 2024-09-26 19:13:24 +02:00
Author
Owner
Copy Link

Unclaimed tickets copy field is unstyled

Thanks: this is fixed now.

> Unclaimed tickets copy field is unstyled Thanks: this is fixed now.
Márton Lente commented 2024-09-26 19:17:39 +02:00
Author
Owner
Copy Link

The horizontal schedule items are a bit too low contrast, they look almost "disabled".

What if we styled the favorite/going items differently?

  • I inreased the horizontal schedule items' colour contrast in dark mode.
  • I like the idea of displaying favourited items differently. I added highlighting styles to schedule colouring mixins with the attribute selector we outlined. See the examples / experiments:

ui-implement-web-assets-theme-system-and-add-dark-theme-3.png

ui-implement-web-assets-theme-system-and-add-dark-theme-4.png

> The horizontal schedule items are a bit too low contrast, they look almost "disabled". > > What if we styled the favorite/going items differently? - I inreased the horizontal schedule items' colour contrast in dark mode. - I like the idea of displaying favourited items differently. I added highlighting styles to schedule colouring mixins with the attribute selector we outlined. See the examples / experiments: ![ui-implement-web-assets-theme-system-and-add-dark-theme-3.png](/attachments/5f6d6385-fd4f-433c-bc31-866e51c1fc18) ![ui-implement-web-assets-theme-system-and-add-dark-theme-4.png](/attachments/17bfb48e-7884-44a5-be03-c1b7b4935950)
ui-implement-web-assets-theme-system-and-add-dark-theme-4.png
676 KiB
ui-implement-web-assets-theme-system-and-add-dark-theme-3.png
676 KiB
❤️ 1
Márton Lente commented 2024-09-26 19:27:51 +02:00
Author
Owner
Copy Link

There is a flicker in loading the theme that is quite noticeable on pages heavy on images, like Speakers, Attendees, or Schedule.

This is not just a problem of our implementation but something many run into when introducing themes.

Some propose using a bit of inline Javascript (vanilla) in the <head>, before anything, which is probably the best. Outlined in this StackExchange answer or this GitHub comment.

This is indeed a problem that is currently present in all project using our Web Assets theme system. Thanks for pointing to the threads. I see the JS has to manage two different things here:

  • Loading the correct colours: This can be done before the DOM has been loaded, and an inline script that calls a function we bundle with Web Assets seems like a good solution.
  • Displaying the correct active theme toggle button: I think this can't be done before the DOM has been loaded, but we can delay the display of the theme toggle button before its correct state is set. For example, it can be statically hidden, that we unhide as the DOM has been loaded to prevent flickering.

I'd open a dedicated issue in Web Assets soon, and address it there. We could deploy the fix as a patch release in all projects using BWA. I think it isn't strictly related to the PR though, but we can do the patch before the merge.

> There is a flicker in loading the theme that is quite noticeable on pages heavy on images, like Speakers, Attendees, or Schedule. > > This is not just a problem of our implementation but something many run into when introducing themes. > > Some propose using a bit of inline Javascript (vanilla) in the `<head>`, before anything, which is probably the best. Outlined in [this StackExchange answer](https://stackoverflow.com/a/62635541/8444393) or [this GitHub comment](https://github.com/vercel/next.js/discussions/12533#discussioncomment-6696252). This is indeed a problem that is currently present in all project using our Web Assets theme system. Thanks for pointing to the threads. I see the JS has to manage two different things here: - **Loading the correct colours:** This can be done before the DOM has been loaded, and an inline script that calls a function we bundle with Web Assets seems like a good solution. - **Displaying the correct active theme toggle button:** _I think_ this can't be done before the DOM has been loaded, but we can delay the display of the theme toggle button before its correct state is set. For example, it can be statically hidden, that we unhide as the DOM has been loaded to prevent flickering. I'd open a dedicated issue in Web Assets soon, and address it there. We could deploy the fix as a patch release in all projects using BWA. I think it isn't strictly related to the PR though, but we can do the patch before the merge.
Márton Lente commented 2024-09-26 19:33:07 +02:00
Author
Owner
Copy Link

The "socket" of unselected filters is still pure white

I addressed, but not sure about this one. For now I set the 'sockets' colours to match button text colours, as they're replaced by checkmarks, that are coloured like this. I think it's okay since they're not strictly 'sockets' (like in radio buttons), but abstract signs representing unselected items. Alternatively I think it's also ok to just hide checkmarks and sockets, as active and inactive states are already managed by colours visually.

> The "socket" of unselected filters is still pure white I addressed, but not sure about this one. For now I set the 'sockets' colours to match button text colours, as they're replaced by checkmarks, that are coloured like this. I think it's okay since they're not strictly 'sockets' (like in radio buttons), but abstract signs representing unselected items. Alternatively I think it's also ok to just hide checkmarks and sockets, as active and inactive states are already managed by colours visually.
Márton Lente added 1 commit 2024-09-27 15:10:20 +02:00 
See the release notes for details.
Part of #103972
Márton Lente added 1 commit 2024-09-27 15:24:50 +02:00 
Implement Web Assets utility JavaScript snippet init-theme introduced in
v2.0.0-alpha.42 to initialize the theme system before page load and prevent
flickering.
Márton Lente force-pushed ui/theme-system from aace9653cb to 42cf6d703a 2024-09-27 15:26:21 +02:00 Compare
Márton Lente commented 2024-09-27 15:31:33 +02:00
Author
Owner
Copy Link

There is a flicker in loading the theme that is quite noticeable on pages heavy on images, like Speakers, Attendees, or Schedule.

This is not just a problem of our implementation but something many run into when introducing themes.

@pablovazquez fickering has been resolved within the framework of issue #94706 in Web Assets. It has been implemented here, and no flickering should happen from now onwards.

Tested with 'artificial' throttling simulating slow networks, and looks good to me. 🤞

> There is a flicker in loading the theme that is quite noticeable on pages heavy on images, like Speakers, Attendees, or Schedule. > > This is not just a problem of our implementation but something many run into when introducing themes. @pablovazquez fickering has been resolved within the framework of issue [#94706](https://projects.blender.org/infrastructure/web-assets/issues/94706) in Web Assets. It has been implemented here, and no flickering should happen from now onwards. Tested with 'artificial' throttling simulating slow networks, and looks good to me. 🤞
❤️ 1
Pablo Vazquez requested changes 2024-09-27 16:01:33 +02:00
Pablo Vazquez left a comment
Owner
Copy Link

Thank you for tackling all issues! Love the followed/assisting items in the schedule.

Thank you for tackling all issues! Love the followed/assisting items in the schedule.
❤️ 1
conference_main/static/conference_main/styles/_schedule.sass
@ -1,0 +7,4 @@
--schedule-filters-container-location-color-l: 60%
.schedule-container
&.horizontal
Pablo Vazquez commented 2024-09-27 16:01:27 +02:00
Owner
Copy Link

Any reason to limit this to the horizontal layout? The same concept could be used in the vertical schedule.

Any reason to limit this to the horizontal layout? The same concept could be used in the vertical schedule.
Márton Lente commented 2024-09-30 14:49:20 +02:00
Author
Owner
Copy Link

Sorry for the late reponse, just noticed the new comments.

I like the idea to display followed/assisting items in the vertical schedule more remarkably, but I think here it should look different, as the vertical layout doesn't have the coloured rows by location. Maybe we should just apply a slightly different tone of grey to highlighted items' backgrounds? Or shall we try to go for location-based colouring here as well, possibly with less saturated tones?

Based on this, I think the existing rule is correct, and we could extend vertical layout styles with additional rules.

Sorry for the late reponse, just noticed the new comments. I like the idea to display followed/assisting items in the vertical schedule more remarkably, but I think here it should look different, as the vertical layout doesn't have the coloured rows by location. Maybe we should just apply a slightly different tone of grey to highlighted items' backgrounds? Or shall we try to go for location-based colouring here as well, possibly with less saturated tones? Based on this, I think the existing rule is correct, and we could extend vertical layout styles with additional rules.
conference_main/static/conference_main/styles/_schedule.sass Outdated
@ -1,0 +15,4 @@
backdrop-filter: brightness(80%)
/* Event favourited styles */
&:has(button[data-is-checked="data-is-checked"])
Pablo Vazquez commented 2024-09-27 16:00:46 +02:00
Owner
Copy Link

Seems that when the button is pressed, it only adds the data-is-checked attribute but it doesn't assign any value. So the user has to reload the page for it to update in real time.

Perhaps we could replace the check:

From:
&:has(button[data-is-checked="data-is-checked"])

To:
&:has(button[data-is-checked])

So it updates as it is pressed.

Seems that when the button is pressed, it only adds the `data-is-checked` attribute but it doesn't assign any value. So the user has to reload the page for it to update in real time. Perhaps we could replace the check: From: `&:has(button[data-is-checked="data-is-checked"])` To: `&:has(button[data-is-checked])` So it updates as it is pressed.
Márton Lente commented 2024-09-30 14:56:17 +02:00
Author
Owner
Copy Link

Thanks, this is indeed much nicer with the less specificity, and works without page reload! 👍 I updated accordingly.

Thanks, this is indeed much nicer with the less specificity, and works without page reload! 👍 I updated accordingly.
martonlente marked this conversation as resolved
Márton Lente added 1 commit 2024-09-30 14:53:35 +02:00 
Remove favourited event style selector's attribute specificity to let style
prevail without page reload, when marking an item as favourited.
Part of #103972
Márton Lente added 1 commit 2024-09-30 14:57:39 +02:00
Márton Lente commented 2024-09-30 15:00:16 +02:00
Author
Owner
Copy Link

Any reason to limit this to the horizontal layout? The same concept could be used in the vertical schedule.

@pablovazquez I'm merging the pull request as discussed IRL. I think this update can be done on main, as it isn't coupled with the theme-system update.

> Any reason to limit this to the horizontal layout? The same concept could be used in the vertical schedule. @pablovazquez I'm merging the pull request as discussed IRL. I think this update can be done on `main`, as it isn't coupled with the theme-system update.
👍 1
Márton Lente closed this pull request 2024-09-30 15:00:17 +02:00
Márton Lente reopened this pull request 2024-09-30 15:00:35 +02:00
Márton Lente merged commit 0d0b3322a2 into main 2024-09-30 15:03:39 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Pablo Vazquez
Labels
Clear labels   
legacy project
Infrastructure: Websites

  
Priority
High

  
Priority
Normal

  
Priority
Unbreak Now!

  
Status
Archived

  
Status
Confirmed

  
Status
Resolved

  
Type
Bug

  
Type
Design

  
Type
Report

  
Type
To Do

No Label
legacy project
Infrastructure: Websites
Priority
High
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Resolved
Type
Bug
Type
Design
Type
Report
Type
To Do
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 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: infrastructure/conference-website#103972
No description provided.
Delete Branch "ui/theme-system"

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?