Skip to content

feat(title): support dynamic type - #27990

Merged
thetaPC merged 21 commits into
FW-4146from
FW-4442
Aug 30, 2023
Merged

feat(title): support dynamic type#27990
thetaPC merged 21 commits into
FW-4146from
FW-4442

Conversation

@thetaPC

@thetaPC thetaPC commented Aug 14, 2023

Copy link
Copy Markdown
Contributor

Issue number: internal


What is the current behavior?

The title component does not support dynamic type.

What is the new behavior?

  • Convert fonts to rem
  • Added scaling tests
  • Large sized titles (size="large") are no longer rendered with position="absolute"
    • Otherwise, the element with larger font size will display as being cutoff
    • position="absolute" is used to center aligned the text. However, a title component with size="large" is always left aligned and doesn't need to be set with absolute.
    • The snapshots are few pixels different because the title component with size="large" is no longer set with position="absolute". This causes the component to lower itself within the box instead of being able to render at the very left top corner.

A few notes on the screenshot diffs:

1px change for Firefox diffs

The accordion and header condense screenshots have a diff for Firefox only. This is due to a rendering quirk in Gecko, the engine that powers Firefox.

Previously, ion-title was positioned absolutely, so it was taking on the dimensions of the parent element. Now that it is positioned statically, ion-title will use the size of its contents to calculate the dimensions. In this case, the height will be determined by the height of the title text. Chrome and Safari render the text as 40px tall, but Firefox renders it as 41px tall.

chrome safari firefox
image image image

Title characters dimensions change

We reduced the padding on the start/end and increased the padding on the top/bottom which is going to cause the title to change dimensions, so these screenshots are correct. Because the dimensions of the title itself are changing too with the absolute --> static change the overall size of the toolbar never actually changes.

Note that we are also moving padding from the toolbar to the title itself now that it is no longer absolutely positioned.

FW-4146 branch
image image

Does this introduce a breaking change?

  • Yes
  • No

Other information

N/A

@bolt-new-by-stackblitz

Copy link
Copy Markdown

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@github-actions github-actions Bot added the package: core @ionic/core package label Aug 14, 2023
@thetaPC
thetaPC marked this pull request as ready for review August 17, 2023 18:21
@thetaPC
thetaPC requested a review from liamdebeasi as a code owner August 17, 2023 18:21
@liamdebeasi

Copy link
Copy Markdown
Contributor

I resolved the merge conflicts with FW-4146 and am running the update screenshot job now.

@sean-perkins
sean-perkins self-requested a review August 22, 2023 21:16
min-width: 100%;

font-size: 34px;
font-size: dynamic-font-min(1, 34px);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to set a minimum font size. Did you mean to use dynamic-font-max instead?

position: relative;

font-size: 13px;
font-size: dynamic-font-min(1, 13px);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to set a minimum font size. Did you mean to use dynamic-font-max instead?

transform: translateZ(0);

font-size: 17px;
font-size: dynamic-font-min(1, 17px);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to set a minimum font size. Did you mean to use dynamic-font-max instead?

transform: translateZ(0);

font-size: 17px;
font-size: dynamic-font-max(17px, 1.2);

@thetaPC thetaPC Aug 29, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used the Contacts app as a reference.

Default
Screenshot 2023-08-29 at 5 02 28 PM

AX5
Screenshot 2023-08-29 at 4 49 54 PM

min-width: 100%;

font-size: 34px;
font-size: dynamic-font-max(34px, 1.8);

@thetaPC thetaPC Aug 29, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used the Contacts app as a reference.

Default
Screenshot 2023-08-29 at 5 01 22 PM

AX5
Screenshot 2023-08-29 at 4 50 28 PM

position: relative;

font-size: 13px;
font-size: dynamic-font-max(13px, 1.8);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used the Contacts app as a reference.

Default
Screenshot 2023-08-29 at 5 00 42 PM

AX5
Screenshot 2023-08-29 at 4 59 19 PM

position: relative;

font-size: 13px;
font-size: dynamic-font-max(13px, 1.8);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weirdly, the small sized title on native has a larger font size when on ax5.

@liamdebeasi liamdebeasi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!

@thetaPC
thetaPC merged commit a23c122 into FW-4146 Aug 30, 2023
@thetaPC
thetaPC deleted the FW-4442 branch August 30, 2023 15:15
@sean-perkins
sean-perkins removed their request for review August 30, 2023 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: core @ionic/core package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants