[To Main] DESENG-630 - Create header section for new engagement page #2539

NatSquared · 2024-06-12T00:03:55Z

Description of changes:

Feature Add new header to "new look" engagement page
- Added a new hero image to the new engagement page
- Added a header overlaying the hero image
  - The header uses the sponsor name, CTA message, and CTA URL from DESENG-629, as well as the engagement title, dates, and status
- Added new engagement status chips for use in the header
- Added link from the old engagement page offering the option to preview the new engagement page
- Added global width limit to the public view to avoid display issues on very wide screens
Task De-duplicate link code and only use <Link>s from the common/Navigation module

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the met-public license (Apache 2.0).

codecov-commenter · 2024-06-12T00:27:52Z

Codecov Report

Attention: Patch coverage is 76.92308% with 12 lines in your changes missing coverage. Please review.

Project coverage is 75.98%. Comparing base (141efd6) to head (c2aee83).
Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2539   +/-   ##
=======================================
  Coverage   75.97%   75.98%           
=======================================
  Files         603      603           
  Lines       21784    21800   +16     
  Branches     1671     1678    +7     
=======================================
+ Hits        16551    16564   +13     
- Misses       4970     4974    +4     
+ Partials      263      262    -1

Flag	Coverage Δ
metweb	`64.55% <76.92%> (+0.02%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
met-web/src/components/common/Input/Button.tsx	`94.80% <ø> (ø)`
met-web/src/components/common/Input/index.tsx	`100.00% <ø> (ø)`
met-web/src/components/common/Navigation/Link.tsx	`100.00% <100.00%> (ø)`
met-web/src/components/common/Typography/index.tsx	`100.00% <100.00%> (ø)`
...engagement/view/EngagementBanner/BannerSection.tsx	`100.00% <100.00%> (ø)`
...mponents/engagement/view/widgets/Map/MapWidget.tsx	`34.88% <100.00%> (+1.55%)`	⬆️
met-web/src/components/common/Typography/Body.tsx	`76.47% <66.66%> (+12.83%)`	⬆️
met-web/src/components/common/Layout/index.tsx	`75.00% <50.00%> (-10.72%)`	⬇️
.../src/components/engagement/view/EngagementView.tsx	`77.41% <81.81%> (+0.49%)`	⬆️
...t-web/src/components/common/Typography/Headers.tsx	`73.07% <70.83%> (-1.93%)`	⬇️

... and 1 file with indirect coverage changes

Baelx

Thanks for this! I always appreciate your attention to detail when it comes to the user experience.

Let's discuss a few things related to accessibility. We can chat further if you like but I think we should:

remove the header tag from the hero image section (make it a labelled section instead)
not open an external link for the CTA

I'd also be curious what the screen reader experience is like for the a tag with a button inside it. If the button is just there for styling then I'd probably recommend we remove it in favour of styling the a tag, but curious to hear why it's there

met-web/src/components/engagement/new/view/EngagementHero.tsx

Baelx · 2024-06-12T18:35:18Z

met-web/src/components/engagement/new/view/EngagementHero.tsx

+                    tabIndex={-1}
+                    target={isExternalCTA ? '_blank' : undefined}
+                >
+                    <Button


Is this button in here purely for styling? So that the link appears as a Met Design system button?

Yes; I prefer using anchor elements for navigation where possible because it helps communicate the intended location to the user. For example, using an onclick => navigate() call does not display the URL on hover. Users can also control-click anchors to open links in a new tab if they choose.

Nat and I discussed this offline: I still feel we should use anchor tags. Nat may try to use the MUI button api to make it an anchor tag to remedy this

Baelx · 2024-06-12T18:42:11Z

met-web/src/components/engagement/new/view/EngagementHero.tsx

+                <a
+                    href={engagement.cta_url || '#cta-section'}
+                    tabIndex={-1}
+                    target={isExternalCTA ? '_blank' : undefined}


Was there specific direction on opening external links in new tabs? From WCAG, "In general, it is better not to open new windows and tabs since they can be disorienting for people, especially people who have difficulty perceiving visual content..."

If we must open in a new tab, we should tell people we're going to do so. One idiomatic way to do this is with an external link icon.

For more info, see: https://www.w3.org/TR/WCAG20-TECHS/G200.html

Yes, I recieved specific directions from UX to open external URLs in new tabs. I believe this is so the user doesn't lose their spot in the app. For internal (anchor) links, there is no new tab behaviour.

After some discussion with Jess and Nat, we'll be opening the link in the same tab, per accessibility guidelines

Baelx · 2024-06-12T18:43:07Z

met-web/src/components/engagement/new/view/EngagementHero.tsx

+                        iconPosition="right"
+                        sx={{ borderRadius: '8px' }}
+                    >
+                        {engagement.cta_message || 'Learn more'}


Nice attention to detail with the fallback message!

Baelx · 2024-06-12T18:54:09Z

met-web/src/components/engagement/new/view/index.tsx


 export const ViewEngagement = () => {
    const { slug, language } = useParams();
    const oldLink = `/${slug}/${language}`;
+    const { engagement } = useLoaderData() as { engagement: Engagement };
    return (
        <>
            <header>


When a <header></header> is adjacent to the main tag, screen readers consider it a "banner" landmark. This means that most users will expect it to contain the site logo, site search tool, and maybe a navigation bar. I'd recommend just sticking with the existing section tag as far as semantic HTML goes.

https://www.w3.org/WAI/ARIA/apg/patterns/landmarks/examples/banner.html

Baelx · 2024-06-12T18:54:55Z

met-web/src/components/engagement/new/view/index.tsx

+                    <section id="cta-section">
+                        {/* TODO: create dark theme provider for this area*/}
+                        <BodyText sx={{ color: colors.surface.white }}>
+                            You are viewing a <b>preview</b> of the new engagement page. It will replace the old page


That's nice that you added this in case users find this and get confused :)

Baelx · 2024-06-12T18:55:55Z

met-web/src/components/engagement/view/EngagementView.tsx

@@ -79,6 +86,17 @@ export const EngagementView = () => {
                <Grid item xs={12}>
                    <EngagementBanner />
                </Grid>
+                {!isLoggedIn && (


Maintaining a nice user experience while we're renovating :)

Baelx · 2024-06-12T18:58:08Z

met-web/src/routes/UnauthenticatedRoutes.tsx

-                <Route path=":slug/:language" element={<ViewEngagement />} />
+                <Route
+                    path=":slug/:language"
+                    loader={async ({ params }: { params: Params<string> }) => {


Nice - starting to implement loaders!

Co-authored-by: Baelx <[email protected]>

sonarcloud · 2024-06-12T22:54:08Z

Quality Gate passed

Issues
0 New issues
1 Accepted issue

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Baelx

Thanks for all the changes!

Baelx · 2024-06-12T22:49:18Z

met-web/src/components/engagement/new/view/EngagementHero.tsx

+                <a
+                    href={engagement.cta_url || '#cta-section'}
+                    tabIndex={-1}
+                    target={isExternalCTA ? '_blank' : undefined}


After some discussion with Jess and Nat, we'll be opening the link in the same tab, per accessibility guidelines

NatSquared added 3 commits June 11, 2024 17:02

Initial commit

e61897f

Remove unused imports

77e0e45

Resolve test error

f288a45

NatSquared marked this pull request as ready for review June 12, 2024 00:31

NatSquared requested a review from Baelx June 12, 2024 18:58

Baelx requested changes Jun 12, 2024

View reviewed changes

NatSquared and others added 5 commits June 12, 2024 14:00

Apply suggestions from code review

1051caf

Co-authored-by: Baelx <[email protected]>

Semantic dates, accessibility, and UX affordances

fcf0fa8

Change button to use anchor element as root

3c5cf00

Remove _blank target from CTA button (stay in the same window)

b576c10

Lower EngagementHero element into <main> landmark

c2aee83

Baelx approved these changes Jun 12, 2024

View reviewed changes

NatSquared merged commit 8512584 into main Jun 12, 2024
8 checks passed

NatSquared deleted the feature/DESENG-630-new-look-header-section branch June 12, 2024 23:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[To Main] DESENG-630 - Create header section for new engagement page #2539

[To Main] DESENG-630 - Create header section for new engagement page #2539

NatSquared commented Jun 12, 2024

codecov-commenter commented Jun 12, 2024 •

edited

Loading

Baelx left a comment

Baelx Jun 12, 2024

NatSquared Jun 12, 2024 •

edited

Loading

Baelx Jun 12, 2024

Baelx Jun 12, 2024

NatSquared Jun 12, 2024 •

edited

Loading

Baelx Jun 12, 2024

Baelx Jun 12, 2024

Baelx Jun 12, 2024

Baelx Jun 12, 2024

Baelx Jun 12, 2024

Baelx Jun 12, 2024

sonarcloud bot commented Jun 12, 2024

Baelx left a comment

Baelx Jun 12, 2024

[To Main] DESENG-630 - Create header section for new engagement page #2539

[To Main] DESENG-630 - Create header section for new engagement page #2539

Conversation

NatSquared commented Jun 12, 2024

codecov-commenter commented Jun 12, 2024 • edited Loading

Codecov Report

Baelx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NatSquared Jun 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NatSquared Jun 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Jun 12, 2024

Quality Gate passed

Baelx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Jun 12, 2024 •

edited

Loading

NatSquared Jun 12, 2024 •

edited

Loading

NatSquared Jun 12, 2024 •

edited

Loading