-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mini profiles tweaks #12919
base: main
Are you sure you want to change the base?
Mini profiles tweaks #12919
Conversation
Size Change: 0 B Total Size: 958 kB ℹ️ View Unchanged
|
6691aba
to
4dd15c8
Compare
4dd15c8
to
0b0ccb4
Compare
@@ -490,6 +492,7 @@ export const renderElement = ({ | |||
editionId={editionId} | |||
RenderArticleElement={RenderArticleElement} | |||
isLastElement={index === totalElements - 1} | |||
sectioned={!!isSectionedMiniProfilesArticle} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does isSectionedMiniProfilesArticle
need to be coerced to a boolean if it's already a boolean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, it's optional. So it could be undefined.
@@ -204,3 +205,16 @@ export const WithSeparatorLine = { | |||
}, | |||
decorators: [centreColumnDecorator], | |||
} satisfies Story; | |||
|
|||
export const Sectioned = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might just be me, but sectioned
as a term is unclear to me. Does it mean it's in a section?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't tested but code LGTM.
What does this change?
Three separate tweaks to the mini profiles format:
h3
s (with font size24px
) for mini profiles articles which include sections. Continue to render titles ash2
s for mini profiles articles which don't include sections. This should help distinguish titles from section headers. More info here (see "Option 2").<p></p>
was considered non-empty.Why?
This was in response to feedback from Rich and Theresa
Screenshots