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

UI: Conference website web-assets v2 upgrade #103970

Merged
Márton Lente merged 50 commits from ui/web-assets-v2-update into main 2024-09-23 12:34:43 +02:00
Conversation 20 Commits 50 Files Changed 47 +520 -520
Márton Lente commented 2024-09-17 15:43:33 +02:00
Owner
Copy Link

This pull request upgrades Git submodule Web Assets to v2, resolves post-upgrade issues, and makes the necessary project-specific changes.

Web Assets v2 brings UI fixes, improvements, and new development features to the platform as detailed on the Web Assets v2 guidelines and notes wiki page.

Important:
Some account screens / states are difficult to trigger locally for visual testing. As such, a more in-depth review is needed especially for these pages with dummy content e.g. on staging, to make sure all components look as expected before the PR can be merged and deployed.

This pull request upgrades Git submodule Web Assets to v2, resolves post-upgrade issues, and makes the necessary project-specific changes. Web Assets v2 brings UI fixes, improvements, and new development features to the platform as detailed on the Web Assets [v2 guidelines and notes](https://projects.blender.org/infrastructure/web-assets/wiki/v2-guidelines-and-notes.md) wiki page. **Important:** Some _account_ screens / states are difficult to trigger locally for visual testing. As such, a more in-depth review is needed especially for these pages with dummy content e.g. on staging, to make sure all components look as expected before the PR can be merged and deployed.
Márton Lente added 11 commits 2024-09-17 15:43:35 +02:00 
Run post web-assets v2 upgrade script bash helper and make project-specific
replacements adjustments base.
Fix background colours after web-assets v2 upgrade, with variable and markup
changes.
Basic conversion of explicit sizing values to web-assets v2 modular sizing, or
rem-based sizing from pixels equivalents.
Change template presentations cards components to box to improve layout
consistency and simplify markup after web-assets v2 upgrade.
Márton Lente added 1 commit 2024-09-17 16:58:02 +02:00
Márton Lente added 1 commit 2024-09-17 17:11:19 +02:00
Márton Lente added 1 commit 2024-09-17 18:20:49 +02:00
Márton Lente added 1 commit 2024-09-17 18:21:38 +02:00
Márton Lente added 1 commit 2024-09-17 18:39:50 +02:00 
Fix remove table rendering when no entries are present in object_list.
Márton Lente added 1 commit 2024-09-17 18:40:43 +02:00 
Fix remove table rendering when no tickets are present in object_list.
Márton Lente added 1 commit 2024-09-18 11:01:14 +02:00
Márton Lente added 1 commit 2024-09-18 12:13:31 +02:00
Márton Lente added 1 commit 2024-09-18 15:35:48 +02:00
Márton Lente added 1 commit 2024-09-18 15:57:13 +02:00 
Apply web-assets v2 form-control styles to immutabe, immutable rendered
classes coming from the back-end.
Márton Lente added 1 commit 2024-09-18 16:00:41 +02:00
Márton Lente added 1 commit 2024-09-18 16:01:28 +02:00
Márton Lente added 1 commit 2024-09-18 16:08:43 +02:00
Márton Lente added 1 commit 2024-09-18 16:50:19 +02:00
Márton Lente added 1 commit 2024-09-18 16:59:56 +02:00
Márton Lente added 1 commit 2024-09-18 17:29:31 +02:00
Márton Lente added 1 commit 2024-09-19 10:36:53 +02:00
Márton Lente added 1 commit 2024-09-19 11:35:25 +02:00
Márton Lente added 1 commit 2024-09-19 11:36:40 +02:00
Márton Lente added 1 commit 2024-09-19 11:42:16 +02:00
Márton Lente added 1 commit 2024-09-19 11:44:07 +02:00
Márton Lente added 1 commit 2024-09-19 11:51:29 +02:00
Márton Lente changed title from WIP: Conference website web-assets v2 upgrade to UI: Conference website web-assets v2 upgrade 2024-09-19 12:16:02 +02:00
Márton Lente requested review from Pablo Vazquez 2024-09-19 12:16:54 +02:00
Pablo Vazquez commented 2024-09-19 12:37:06 +02:00
Owner
Copy Link

I get a crash on loading the landing page locally, it's assuming edition.header.

crash
<div style="background-image: url({{ edition.header.url }}); background-position-y: 50%" class="hero-bg"></div>

Needs a check if edition.header exists.

I get a crash on loading the landing page locally, it's assuming `edition.header`. ![crash](/attachments/b41bc1a4-8e75-4ed3-9783-888c0c11102b) `<div style="background-image: url({{ edition.header.url }}); background-position-y: 50%" class="hero-bg"></div>` Needs a check if `edition.header` exists.
image.png
166 KiB
image.png
58 KiB
image.png
Pablo Vazquez commented 2024-09-19 12:40:32 +02:00
Owner
Copy Link

We already do that check everywhere btw. Perhaps the hero background could become a reusable component.

if check

We already do that check everywhere btw. Perhaps the hero background could become a reusable component. ![if check](/attachments/ca2afa85-bf9e-4849-884d-87c7190d681f)
👍 1
Márton Lente added 1 commit 2024-09-19 12:43:09 +02:00 
Part of #103970
Márton Lente commented 2024-09-19 12:45:08 +02:00
Author
Owner
Copy Link

I get a crash on loading the landing page locally, it's assuming edition.header.

crash
<div style="background-image: url({{ edition.header.url }}); background-position-y: 50%" class="hero-bg"></div>

Needs a check if edition.header exists.

Thanks, it has been fixed locally.

In the meanwhile I'm checking if we can turn it into a reusable component.

> I get a crash on loading the landing page locally, it's assuming `edition.header`. > > ![crash](/attachments/b41bc1a4-8e75-4ed3-9783-888c0c11102b) > `<div style="background-image: url({{ edition.header.url }}); background-position-y: 50%" class="hero-bg"></div>` > > Needs a check if `edition.header` exists. > Thanks, it has been fixed locally. In the meanwhile I'm checking if we can turn it into a reusable component.
Pablo Vazquez commented 2024-09-19 16:00:01 +02:00
Owner
Copy Link

Tickets boxes do not have a background (products_table.html)

http://conference.local:8009/tickets/overview/

tickets

Tickets boxes do not have a background (`products_table.html`) http://conference.local:8009/tickets/overview/ ![tickets](/attachments/d4e50523-b495-41dc-b56e-2c27d071e60d)
image.png
228 KiB
👍 1
Pablo Vazquez commented 2024-09-19 16:07:04 +02:00
Owner
Copy Link

Top-bar accent button styling is lost:

PR Expected
PR expected
Top-bar accent button styling is lost: |PR|Expected| |----|----| |![PR](/attachments/984ed667-0d19-41e3-b9b1-959cdf9caaaf)|![expected](/attachments/e0fbda41-f54c-4254-8705-6c94945cd4d5)|
image.png
11 KiB
image.png
13 KiB
Márton Lente commented 2024-09-19 16:11:52 +02:00
Author
Owner
Copy Link

Tickets boxes do not have a background (products_table.html)

Thanks, this has been fixed.

> Tickets boxes do not have a background (`products_table.html`) Thanks, this has been fixed.
Márton Lente commented 2024-09-19 16:15:10 +02:00
Author
Owner
Copy Link

Top-bar accent button styling is lost:

PR Expected
PR expected

Web Assets' bundled navbar component has a default CTA button, with the above (more modest) styling – the new styling comes from here. If we want to make the button more prominent in the navbar, should we maybe change it in BWA to match the styling we currently have on the Conference website (so that to make the contrasty CTA button the Web Assets default)?

> Top-bar accent button styling is lost: > > |PR|Expected| > |----|----| > |![PR](/attachments/984ed667-0d19-41e3-b9b1-959cdf9caaaf)|![expected](/attachments/e0fbda41-f54c-4254-8705-6c94945cd4d5)| Web Assets' bundled navbar component has a default CTA button, with the above (more modest) styling – the new styling comes from here. If we want to make the button more prominent in the navbar, should we maybe change it in BWA to match the styling we currently have on the Conference website (so that to make the contrasty CTA button the Web Assets default)?
Pablo Vazquez commented 2024-09-19 16:20:10 +02:00
Owner
Copy Link

Web Assets' bundled navbar component has a default CTA button, with the above (more modest) styling – the new styling comes from here.

I think the root of the issue is that the Buy Tickets looks like btn-primary when it should be btn-accent. We don't always want all buttons to be this prominent. Both primary and accent are valid.

> Web Assets' bundled navbar component has a default CTA button, with the above (more modest) styling – the new styling comes from here. I think the root of the issue is that the Buy Tickets looks like `btn-primary` when it should be `btn-accent`. We don't always want all buttons to be this prominent. Both `primary` and `accent` are valid.
👍 1
Pablo Vazquez commented 2024-09-19 16:23:58 +02:00
Owner
Copy Link

Styling differences in the user profile page.

  • "Dim" text is no longer dim. Everything seems to be of the same value (see Sessions header, or user's details)
  • Avatar size is now smaller than it used to be.
  • Bio text is too large (we can probably just remove the .lead class here, it just looks out of place now)
PR Expected
pr expected

The lack of "dim" text is also visible in the footer links:

PR
PR
Expected
expected
Styling differences in the user profile page. * "Dim" text is no longer dim. Everything seems to be of the same value (see Sessions header, or user's details) * Avatar size is now smaller than it used to be. * Bio text is too large (we can probably just remove the `.lead` class here, it just looks out of place now) |PR|Expected| |----|----| |![pr](/attachments/85c4fe63-07fa-4127-bc3c-58478406b577)|![expected](/attachments/84aa62b1-3fed-43a0-88e8-ff57bbc268b9)| The lack of "dim" text is also visible in the footer links: |PR| |----| |![PR](/attachments/8b0d7204-dac2-4e86-b22d-5fa81cdebb2f)| |Expected| |----| |![expected](/attachments/52a8694f-6358-4a13-af45-ed1e9d091883)|
image.png
215 KiB
image.png
204 KiB
image.png
15 KiB
image.png
15 KiB
👍 1
Pablo Vazquez commented 2024-09-19 16:25:51 +02:00
Owner
Copy Link

Crash on presentation review pages.

http://conference.local:8009/2023/presentations/1911/review/

crash

Crash on presentation review pages. http://conference.local:8009/2023/presentations/1911/review/ ![crash](/attachments/8c1cd88e-9929-439a-b432-7ac9b0c361a8)
image.png
98 KiB
Pablo Vazquez commented 2024-09-19 16:29:15 +02:00
Owner
Copy Link

Broken active day button styling in schedule.

https://conference.blender.org/2022/schedule/?display=horizontal

PR Expected
PR expected
Broken active day button styling in schedule. https://conference.blender.org/2022/schedule/?display=horizontal |PR|Expected| |----|----| |![PR](/attachments/7271dc44-b7aa-4687-9e05-968b1477c0a5)|![expected](/attachments/c39fbc77-767c-46bd-b73b-f4ac2797dab4)|
image.png
117 KiB
image.png
132 KiB
Pablo Vazquez commented 2024-09-19 16:41:11 +02:00
Owner
Copy Link

The jump-to-top button has a huge shadow compared to production.

PR Expected
PR Expected
The jump-to-top button has a huge shadow compared to production. |PR|Expected| |----|----| |![PR](/attachments/5ccdba01-3689-4a89-8fb8-d7b34c8e244d)|![Expected](/attachments/df602907-922c-42c4-bfd2-9b05b7b0bef6)|
image.png
5.5 KiB
image.png
7.6 KiB
Márton Lente added 1 commit 2024-09-19 17:05:25 +02:00 
Part of #103970
Márton Lente added 1 commit 2024-09-19 17:06:15 +02:00
Márton Lente force-pushed ui/web-assets-v2-update from 1aed7d9a11 to 27f4f928ff 2024-09-19 17:07:12 +02:00 Compare
Márton Lente added 1 commit 2024-09-19 17:07:41 +02:00 
See Release notes for details.
Part of #103970
Márton Lente added 1 commit 2024-09-19 17:07:58 +02:00 
Make navbar btn cta more prominent.
Part of #103970
Márton Lente commented 2024-09-19 17:14:54 +02:00
Author
Owner
Copy Link

Web Assets' bundled navbar component has a default CTA button, with the above (more modest) styling – the new styling comes from here.

I think the root of the issue is that the Buy Tickets looks like btn-primary when it should be btn-accent. We don't always want all buttons to be this prominent. Both primary and accent are valid.

This makes a lot of sense, but was a bit tricky, as the navbar component has its own button styles to enable Web-Assets-style buttons in projects that don't use BWA as their framework, which somewhat clashed. I resolved this in https://projects.blender.org/infrastructure/web-assets/releases/tag/v2.0.0-alpha.35, that enables default BWA buttons to be used in navbar (and changed to btn-accent here accordingly).

> > Web Assets' bundled navbar component has a default CTA button, with the above (more modest) styling – the new styling comes from here. > > I think the root of the issue is that the Buy Tickets looks like `btn-primary` when it should be `btn-accent`. We don't always want all buttons to be this prominent. Both `primary` and `accent` are valid. > This makes a lot of sense, but was a bit tricky, as the `navbar` component has its own button styles to enable Web-Assets-style buttons in projects that don't use BWA as their framework, which somewhat clashed. I resolved this in https://projects.blender.org/infrastructure/web-assets/releases/tag/v2.0.0-alpha.35, that enables default BWA buttons to be used in navbar (and changed to `btn-accent` here accordingly).
Márton Lente added 1 commit 2024-09-19 17:20:45 +02:00 
Part of #103970
Pablo Vazquez commented 2024-09-19 17:22:54 +02:00
Owner
Copy Link

Festival: voting is broken and stars are always styled yellow, don't react on hover.

http://conference.local:8009/2023/festival/entries/556/

PR Expected
Festival: voting is broken and stars are always styled yellow, don't react on hover. http://conference.local:8009/2023/festival/entries/556/ |PR|Expected| |----|----| |<video src="/attachments/cf45009e-4515-4ee8-b3a4-63c65531d44f" title="festival_voting_pr.mov" controls autoplay loop></video>|<video src="/attachments/cab12f7c-570d-4434-acf7-23bbac542efa" title="festival_voting_production.mov" controls autoplay loop></video>|
festival_voting_pr.mov
156 KiB
festival_voting_production.mov
260 KiB
Márton Lente added 1 commit 2024-09-19 17:26:28 +02:00 
Part of #103970
Márton Lente added 1 commit 2024-09-19 17:51:42 +02:00 
Make profile bio text font-size smaller.
Part of #103970
Márton Lente added 1 commit 2024-09-19 18:12:36 +02:00 
Part of #103970
Márton Lente added 1 commit 2024-09-19 18:22:02 +02:00 
Part of #103970
Pablo Vazquez commented 2024-09-19 18:23:46 +02:00
Owner
Copy Link

Review issues:

  • Title <h3> not closed, making the entire description a title.
  • Section titles a bit too large (DESCRIPTION, N MESSAGES)
  • Entire messages discussion missing.

Example: http://conference.local:8009/2021/presentations/1301/review/

Expected
expected
PR
pr
Review issues: * Title `<h3>` not closed, making the entire description a title. * Section titles a bit too large (DESCRIPTION, N MESSAGES) * Entire messages discussion missing. Example: http://conference.local:8009/2021/presentations/1301/review/ |Expected| |----| |![expected](/attachments/18e100dd-ec28-4ad8-bb9f-b595bfb17cbb)| |PR| |----| |![pr](/attachments/4a060860-c441-4b0a-bf2d-7c968af03e45)|
image.png
87 KiB
image.png
80 KiB
👍 1
Pablo Vazquez requested changes 2024-09-19 18:25:49 +02:00
Dismissed
Pablo Vazquez left a comment
Owner
Copy Link

First pass of review

First pass of review
Márton Lente added 1 commit 2024-09-19 18:36:46 +02:00 
Part of #103970
Márton Lente added 1 commit 2024-09-19 18:43:08 +02:00 
Part of #103970
Márton Lente added 1 commit 2024-09-19 18:48:52 +02:00 
Part of #103970
Márton Lente commented 2024-09-19 18:54:17 +02:00
Author
Owner
Copy Link

@pablovazquez thank you for your very thorough feedback! 🙏

All of the above items should be improved or fixed now. I'm reopening the review request.

@pablovazquez thank you for your very thorough feedback! 🙏 All of the above items should be improved or fixed now. I'm reopening the review request.
Márton Lente requested review from Pablo Vazquez 2024-09-19 18:54:26 +02:00
Pablo Vazquez requested changes 2024-09-19 19:03:40 +02:00
Dismissed
Pablo Vazquez left a comment
Owner
Copy Link

The jump-to-top button has a huge shadow compared to production.

PR Expected
PR Expected

This issue is still present.

> The jump-to-top button has a huge shadow compared to production. > > |PR|Expected| > |----|----| > |![PR](/attachments/5ccdba01-3689-4a89-8fb8-d7b34c8e244d)|![Expected](/attachments/df602907-922c-42c4-bfd2-9b05b7b0bef6)| This issue is still present.
Pablo Vazquez commented 2024-09-19 19:04:46 +02:00
Owner
Copy Link

Also visible in elements with shadow-lg like the box in Festival Finals voting page.

festival finals

Also visible in elements with `shadow-lg` like the box in Festival Finals voting page. ![festival finals](/attachments/2cded070-18e2-4657-89a5-e4d9787404af)
image.png
161 KiB
👍 1
Márton Lente added 1 commit 2024-09-20 11:37:06 +02:00 
See Release notes for details.
Part of #103970
Márton Lente added 1 commit 2024-09-20 11:39:28 +02:00 
This reverts commit 54469a8e84.
Márton Lente commented 2024-09-20 11:45:05 +02:00
Author
Owner
Copy Link

Also visible in elements with shadow-lg like the box in Festival Finals voting page.

Thank you. It seems this was a v2 issue on the Web Assets level. All shadow styles have been systematically revised and fixed in v2.0.0-alpha.37 – shadows should now look correct on all pages.

> Also visible in elements with `shadow-lg` like the box in Festival Finals voting page. Thank you. It seems this was a v2 issue on the Web Assets level. All shadow styles have been systematically revised and fixed in [v2.0.0-alpha.37](https://projects.blender.org/infrastructure/web-assets/releases/tag/v2.0.0-alpha.37) – shadows should now look correct on all pages.
Márton Lente requested review from Pablo Vazquez 2024-09-20 11:45:14 +02:00
Pablo Vazquez commented 2024-09-23 10:30:13 +02:00
Owner
Copy Link

A small one, dropdown's z-index seem to conflict with items with fixed positions like the festival vote footer using .fixed-bottom among others:
row fixed-bottom position-sticky bg-light shadow-lg festival-vote-footer

Example (if you add enable the festival and add some movies yourself) http://conference.local:8009/2024/festival/finals/

dropdown zindex

It's not a big deal though because it just happens on long menus which most of the time only admins get anyway. But it'd be good to tackle that in web-assets. Basically have dropdowns z-index be the highest up, over fixed items z-index even.

Also noticed the z-index is not a CSS variable but a fixed number (probably a Sass variable), it might be easier to customize as CSS variable.

fixed bottom

A small one, dropdown's z-index seem to conflict with items with fixed positions like the festival vote footer using `.fixed-bottom` among others: `row fixed-bottom position-sticky bg-light shadow-lg festival-vote-footer` Example (if you add enable the festival and add some movies yourself) http://conference.local:8009/2024/festival/finals/ ![dropdown zindex](/attachments/4e38a787-3d0c-4b90-b973-6d651a093341) It's not a big deal though because it just happens on long menus which most of the time only admins get anyway. But it'd be good to tackle that in web-assets. Basically have dropdowns z-index be the highest up, over fixed items z-index even. Also noticed the z-index is not a CSS variable but a fixed number (probably a Sass variable), it might be easier to customize as CSS variable. ![fixed bottom](/attachments/6ee355ac-5eed-4d3a-bbc7-44e7791a7112)
image.png
132 KiB
image.png
26 KiB
👍 1
Márton Lente added 1 commit 2024-09-23 11:55:21 +02:00 
Fixes fixed elements overlapping the navbar. See Release notes for details.
Part of #103970
Márton Lente added 1 commit 2024-09-23 12:11:00 +02:00
Márton Lente commented 2024-09-23 12:19:27 +02:00
Author
Owner
Copy Link

It's not a big deal though because it just happens on long menus which most of the time only admins get anyway. But it'd be good to tackle that in web-assets. Basically have dropdowns z-index be the highest up, over fixed items z-index even.

Also noticed the z-index is not a CSS variable but a fixed number (probably a Sass variable), it might be easier to customize as CSS variable.

@pablovazquez thank you for reporting this. I resolved the issues in Web Assets the following way:

  • It seems the overlap issues were caused by navbar's z-index properties. As a parent element, it was overlapped by subsequent markup items with same or higher z-indexes. Because of this, its children elements (like dropdowns) couldn't take precedence, even if their local z-indexes were higher. Now it should work as expected.
  • Zindex stack's Sass variables were mapped to existing BWA custom properties through the Bootstrap 5 utilities API, so they'll be present in utility classes from now as well.

With the above changes I think the PR is ready to be merged, and I'm going ahead as discussed IRL.

> It's not a big deal though because it just happens on long menus which most of the time only admins get anyway. But it'd be good to tackle that in web-assets. Basically have dropdowns z-index be the highest up, over fixed items z-index even. > > Also noticed the z-index is not a CSS variable but a fixed number (probably a Sass variable), it might be easier to customize as CSS variable. @pablovazquez thank you for reporting this. I resolved the issues in Web Assets the following way: - It seems the overlap issues were caused by `navbar`'s z-index properties. As a parent element, it was overlapped by subsequent markup items with same or higher z-indexes. Because of this, its children elements (like dropdowns) couldn't take precedence, even if their local z-indexes were higher. Now it should work as expected. - Zindex stack's Sass variables were mapped to existing BWA custom properties through the Bootstrap 5 utilities API, so they'll be present in utility classes from now as well. With the above changes I think the PR is ready to be merged, and I'm going ahead as discussed IRL.
❤️ 1
Márton Lente added 1 commit 2024-09-23 12:24:02 +02:00
Pablo Vazquez approved these changes 2024-09-23 12:28:06 +02:00
Pablo Vazquez left a comment
Owner
Copy Link

Thanks for tackling all the issues!

Thanks for tackling all the issues!
❤️ 1
Márton Lente merged commit 93400ef43c into main 2024-09-23 12:34:43 +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#103970
No description provided.
Delete Branch "ui/web-assets-v2-update"

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?