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

[IOAPPX-419] Adjust the size of Icon, Pictogram and some components based on the value of fontScale #348

Merged
merged 41 commits into from
Dec 16, 2024

Conversation

dmnplb
Copy link
Collaborator

@dmnplb dmnplb commented Nov 6, 2024

Short description

This PR adds a dynamic size to some components, based on the value of fontScale. Dynamic size is currently supported on the following components:

  • Tag, Badge, Alert and FeatureInfo
  • All the selection components (ListItemCheckbox, ListItemRadio, etc…)
  • All the ListItem… components

List of changes proposed in this pull request

  • Add the new useIOFontDynamicScale hook to get the current fontScale value
  • Add the new allowFontScaling to Icon, AnimatedIcon and Pictogram components to enable dynamic size based on the fontScale value
  • Add the new allowScaleSpacing to Stack components to enable the same behavior
  • Add dynamic spacing to Tag, Badge, all the selection and ListItem… components
  • Increase value of the maxFontSizeMultiplier from 1.25 to 1.5
  • Hide decorative icons from ListItem… and Module… components if the text size multiplier is quite big (>= 1.5)

Preview

ListItemCheckbox

As you can see, the size of the margins, icons and checkboxes also changes depending on the value of fontScale:

Default size Larger text size
Simulator Screenshot - iPhone 16 Pro - 2024-11-06 at 17 14 09 Simulator Screenshot - iPhone 16 Pro - 2024-11-06 at 17 12 20

Alert

Same as above, but with boldEnabled set to ON.

Default size Larger text size
Simulator Screenshot - iPhone 16 Pro - 2024-11-07 at 16 26 33 Simulator Screenshot - iPhone 16 Pro - 2024-11-07 at 16 23 46

How to test

  1. Launch the example app
  2. Go to the Accessibility → Display & Text Size → Larger text
  3. Change the values
  4. Go back to the example app to see the applied changes

@dmnplb dmnplb marked this pull request as ready for review November 6, 2024 16:19
@dmnplb dmnplb requested review from a team as code owners November 6, 2024 16:19
@dmnplb dmnplb changed the title [IOAPPX-419] Adjust the size of some components based on the value of fontScale [IOAPPX-419] Adjust the size of Icon, Pictogram and some components based on the value of fontScale Nov 8, 2024
Copy link
Contributor

@LeleDallas LeleDallas left a comment

Choose a reason for hiding this comment

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

Here are some suggestions. Feel free to reach out if you have any questions!

src/components/alert/Alert.tsx Outdated Show resolved Hide resolved
src/components/alert/Alert.tsx Outdated Show resolved Hide resolved
src/components/checkbox/CheckboxLabel.tsx Outdated Show resolved Hide resolved
@dmnplb
Copy link
Collaborator Author

dmnplb commented Nov 25, 2024

@dmnplb don't forget to include fontScale in the android:configChanges attribute of AndroidManifest.xml file to prevent crashes caused by system font size changes

@LeleDallas Addressed in the following commit → 1428067 (#348)

@LeleDallas
Copy link
Contributor

After running some tests, it appears that the app keeps crashing on Android 14.

Screen.Recording.2024-12-04.at.11.56.16.mov

Android 14 vs Android 10

@LeleDallas
Copy link
Contributor

After running some tests, it appears that the app keeps crashing on Android 14.

Screen.Recording.2024-12-04.at.11.56.16.mov
Android 14 vs Android 10

To resolve the issue, add the following code to your MainActivity.java file:

import android.os.Bundle;

@Override
  protected void onCreate(Bundle savedInstanceState) {
      super.onCreate(null);
  }

Thank's @shadowsheep1 for the support!

@dmnplb
Copy link
Collaborator Author

dmnplb commented Dec 5, 2024

To resolve the issue, add the following code to your MainActivity.java file

@LeleDallas We already have this setting in the io-app application, don`t we?

@LeleDallas
Copy link
Contributor

To resolve the issue, add the following code to your MainActivity.java file

@LeleDallas We already have this setting in the io-app application, don`t we?

Yep! I think the effort to put it here is minimal

@dmnplb
Copy link
Collaborator Author

dmnplb commented Dec 5, 2024

Yep! I think the effort to put it here is minimal

@LeleDallas Just added with this commit: 3912dee (#348)

@LeleDallas
Copy link
Contributor

The feature now works as expected without crashing 🎊🎊!

My only concern is about restarting the session after updating the text size...
As shown in the video, if the app is in a particular state during the restart after updating the text size, that state will be lost. However, I’ve noticed that other apps, such as Bluesky, handle it in a similar way.

Untitled.mov

@LeleDallas
Copy link
Contributor

Good on iOS device too!

ScreenRecording_12-06-2024.15-58-11_1.MP4

Copy link
Contributor

@LeleDallas LeleDallas left a comment

Choose a reason for hiding this comment

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

As discussed during the meeting, we find this behavior acceptable 💫

@dmnplb dmnplb merged commit ad30a4a into main Dec 16, 2024
6 checks passed
@dmnplb dmnplb deleted the IOAPPX-419-dynamic-size-based-on-dynamic-type branch December 16, 2024 15:13
dmnplb added a commit that referenced this pull request Dec 17, 2024
…n the `fontScale` value (#356)

> [!caution]
> This PR depends on:
> * #348

## Short description
This PR enables dynamic size on the `SearchInput` and `TextInputBase`
components

## List of changes proposed in this pull request
- Add `allowFontScaling` prop to the icons contained in the both
components
- Add `allowScaleSpacing` prop to `VSpacer` and `HSpacer` components

## How to test
Go to the **Text Inputs** and **Search Input** screens in the example
app
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants