Skip to content
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

PageHeader - Subtitle width #3832

Open
SeamusLeonardHPE opened this issue Apr 10, 2024 · 5 comments
Open

PageHeader - Subtitle width #3832

SeamusLeonardHPE opened this issue Apr 10, 2024 · 5 comments
Assignees
Labels
Phase:Current Sprint Used for issues that are being worked on current sprint. Type:Fix Used in issues for fixing bugs and inconsistencies on existing material. Where:Figma Used in issues related to any of our libraries in Figma.

Comments

@SeamusLeonardHPE
Copy link
Collaborator

SeamusLeonardHPE commented Apr 10, 2024

As highlighted by George in the linked comment.

The figma page header component display different behaviour then its coded grommet counterpart.

The css grid rules for the "subtitle" div contain a min/max 384/768px that helps text wrap at less than 80 characters wide.

Should the same principles apply in the figma component to help product designers create accurate mocks

Image

@SeamusLeonardHPE SeamusLeonardHPE added Type:Fix Used in issues for fixing bugs and inconsistencies on existing material. Where:Figma Used in issues related to any of our libraries in Figma. labels Apr 10, 2024
@SeamusLeonardHPE SeamusLeonardHPE added the Phase:Current Sprint Used for issues that are being worked on current sprint. label Apr 17, 2024
@ashifalinadaf
Copy link
Collaborator

Working on the issue,
WIP FIle

@ashifalinadaf
Copy link
Collaborator

ashifalinadaf commented Apr 19, 2024

  • There is a divergence between how text wrapping behave in figma & code on the page header component
  • Code has an undocumented max width for text (should be in DS guidance - is this enforce elsewhere?)
  • what is the max width for the div, what is the max width for the 'p string'
  • How does this behaviour change when in a smaller responsive size

@ashifalinadaf
Copy link
Collaborator

ashifalinadaf commented Apr 22, 2024

Initial proposals for changes to the Figma component

Changes in the font size
Subtitle - Text/Normal/Xlarge (24/30)
XLarge - No changes
Large - No changes
Medium - No changes
Small - Text/Normal/Xlarge (24/30) to Text/Normal/default (18/24)
Xsmall - Text/Normal/Xlarge (24/30) to Text/Normal/default (18/24)

Title - Heading /Desktop/ h1 medium (36/36)
XLarge - No changes
Large - No changes
Medium - No changes
Small - Heading /Desktop/ h1 medium (36/36) to Heading /Desktop/ h1 small (24/24)
Xsmall - Heading /Desktop/ h1 medium (36/36) to Heading /Desktop/ h1 small (24/24)

Changes in width:
Subtitle:
XL = 768px to 600px
L = 768px to 600px
M = 528px to 600px
S = 384px to 450px
XS = 192px to 450px

Additional guidelines will be added to the Design System Site following the changes made above

@ashifalinadaf
Copy link
Collaborator

ashifalinadaf commented Apr 24, 2024

Created a Pageheader figma component
@SeamusLeonardHPE please let me know your thoughts on this,
I have read all the comments from you and Taylor,
Keeping the button group with all the buttons including the action menu/overflow menu, primary/secondary/default with left and right alignment.

Changes in the font size and width as mentioned above, and also at small and xsmall sizes will have a buttons wrapped below the subtitle.

@ashifalinadaf
Copy link
Collaborator

@SeamusLeonardHPE
please have a look and see if this is okay, please do add comments if there are any changes.
Latest iteration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Phase:Current Sprint Used for issues that are being worked on current sprint. Type:Fix Used in issues for fixing bugs and inconsistencies on existing material. Where:Figma Used in issues related to any of our libraries in Figma.
Projects
None yet
Development

No branches or pull requests

2 participants