[To Main] Feature - DESENG 631: Add Engagement Description Block #2541
Conversation
| @@ -22,13 +24,22 @@ export const BodyText = ({ | |||
| regular: '24px', | |||
| large: '24px', | |||
| }[size]; | |||
| const fontWeight = () => { | |||
There was a problem hiding this comment.
Do you foresee us needing additional font weight customization as time goes on? You could just expose a prop that accepts a valid fontWeight attribute and feeds it right to the corresponding Material UI typography property
There was a problem hiding this comment.
I don't honestly forsee us needing this - if a specific numeric font weight is needed it can be overridden with sx.fontWeight
| export const EngagementDescription = ({ engagement }: { engagement: Engagement }) => { | ||
| const { widgets } = useLoaderData() as { widgets: Widget[] }; | ||
| return ( | ||
| <section id="cta-section"> |
There was a problem hiding this comment.
Could we label this accessibility region? In the last PR, we used an aria-label directly for something descriptive of the section. In this case, I think we can use the H2 below. You'd reference it with an aria-labelledby attribute
See here for the relevant technique: https://www.w3.org/WAI/ARIA/apg/patterns/landmarks/examples/region.html
|
|
||
| export const engagementLoader = async ({ params }: { params: Params<string> }) => { | ||
| const { slug } = params; | ||
| const engagement = getEngagementIdBySlug(slug ?? '').then((response) => getEngagement(response.engagement_id)); |
There was a problem hiding this comment.
Are you meaning to run all these in series? I think without the await keyword, some of these will run asynchronously
There was a problem hiding this comment.
Actually, these promises are intentionally not resolved within the loader - it would cause the route to be slow during navigation. This method is only to construct and gather the relevant promises in one place. The defer() pattern allows us to choose what data we want to await vs. not, finding the right tradeoff between loading time and CLS. Deferred variables (promises) are awaited inside the route itself, using the element from react-router-dom in combination with with a MUI Skeleton fallback.
More reading: Deferred Data
|
Issue #: 🎟️ DESENG-631
Description of changes:
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).