Skip to content

feat(searchbar): support dynamic type - #28043

Merged
liamdebeasi merged 10 commits into
FW-4146from
FW-4440
Aug 28, 2023
Merged

feat(searchbar): support dynamic type#28043
liamdebeasi merged 10 commits into
FW-4146from
FW-4440

Conversation

@liamdebeasi

@liamdebeasi liamdebeasi commented Aug 22, 2023

Copy link
Copy Markdown
Contributor

Issue number: N/A


What is the current behavior?

  • Searchbar does not support Dynamic Type

What is the new behavior?

  • Searchbar supports Dynamic Type
  • Added dynamic type verification test
  • I also found a bug where the text on the iOS searchbar was always accounting for the clear button, even when the clear button was hidden. I fixed this and added a test.

Does this introduce a breaking change?

  • Yes
  • No

Other information

@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 22, 2023
@liamdebeasi liamdebeasi changed the title Fw 4440 feat(searchbar): support dynamic type Aug 24, 2023
@liamdebeasi
liamdebeasi marked this pull request as ready for review August 25, 2023 12:16
@liamdebeasi
liamdebeasi requested review from a team and thetaPC and removed request for a team August 25, 2023 17:20
Comment thread core/src/components/searchbar/searchbar.md.vars.scss

/// @prop - Size of the searchbar input search icon
$searchbar-md-input-search-icon-size: 20px !default;
$searchbar-md-input-search-icon-size: dynamic-font(21px) !default;

@liamdebeasi liamdebeasi Aug 28, 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.

This is an intentional change from 20px to 21px.

The actual width/height of the icon was 21px due to the following code:

width: $searchbar-md-input-search-icon-size + 1;
height: $searchbar-md-input-search-icon-size + 1;

I discussed with Brandy, and these variables were originally used for other things like background size which is why the 1px was not incorporated as part of the sass variable. However, the variable is only used for width/height so I made it part of the variable directly.

@liamdebeasi
liamdebeasi merged commit fc39cd3 into FW-4146 Aug 28, 2023
@liamdebeasi
liamdebeasi deleted the FW-4440 branch August 28, 2023 18:03
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