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

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
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

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.

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)
Márton Lente added 1 commit 2024-09-19 12:43:09 +02:00
Author
Owner

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.

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)
228 KiB

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)|
Author
Owner

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.
Author
Owner

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)?

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.

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)|
Crash on presentation review pages. http://conference.local:8009/2023/presentations/1911/review/ ![crash](/attachments/8c1cd88e-9929-439a-b432-7ac9b0c361a8)

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)|
117 KiB
132 KiB

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)|
5.5 KiB
7.6 KiB
Márton Lente added 1 commit 2024-09-19 17:05:25 +02:00
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
Márton Lente added 1 commit 2024-09-19 17:07:58 +02:00
Author
Owner

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

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>|
Márton Lente added 1 commit 2024-09-19 17:26:28 +02:00
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
Márton Lente added 1 commit 2024-09-19 18:22:02 +02:00

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)|
Pablo Vazquez requested changes 2024-09-19 18:25:49 +02:00
Dismissed
Pablo Vazquez left a comment
Owner

First pass of review

First pass of review
Márton Lente added 1 commit 2024-09-19 18:36:46 +02:00
Márton Lente added 1 commit 2024-09-19 18:43:08 +02:00
Márton Lente added 1 commit 2024-09-19 18:48:52 +02:00
Author
Owner

@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

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.

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)
161 KiB
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
Author
Owner

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

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)
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
Author
Owner

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.
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

Thanks for tackling all the issues!

Thanks for tackling all the issues!
Márton Lente merged commit 93400ef43c into main 2024-09-23 12:34:43 +02:00
Sign in to join this conversation.
No reviewers
No Milestone
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.