Skip to content

feat(toolbar): support dynamic type - #28073

Merged
liamdebeasi merged 4 commits into
FW-4146from
FW-4444
Aug 30, 2023
Merged

feat(toolbar): support dynamic type#28073
liamdebeasi merged 4 commits into
FW-4146from
FW-4444

Conversation

@liamdebeasi

@liamdebeasi liamdebeasi commented Aug 29, 2023

Copy link
Copy Markdown
Contributor

Issue number: N/A


What is the current behavior?

Most of the work to support dynamic type has been handled in individual components such as ion-title, ion-button, and ion-back-button. However, I noticed two issues:

  1. Searchbars in toolbars do not scale with dynamic type.
  2. Toolbar has code that targets an improper usage of ion-back-button. The following code set the height of the ion-back-button when used in a toolbar with a searchbar:
    :host(.toolbar-searchbar) ::slotted(ion-back-button) {
    height: 38px;
    }

However, this code should never apply because ion-back-button should be slotted inside of ion-buttons not ion-back-button. Slotting ion-back-button directly inside of ion-toolbar causes the back button and searchbar to appear on different lines.

What is the new behavior?

  • Updated searchbar to scale in a toolbar
  • Added test for new searchbar behavior
  • Removed code targeting improper usage

Does this introduce a breaking change?

  • Yes
  • No

Other information

@github-actions github-actions Bot added the package: core @ionic/core package label Aug 29, 2023
@bolt-new-by-stackblitz

Copy link
Copy Markdown

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

@liamdebeasi liamdebeasi changed the title Fw 4444 feat(toolbar): support dynamic type Aug 29, 2023
@liamdebeasi
liamdebeasi marked this pull request as ready for review August 29, 2023 16:47
@liamdebeasi
liamdebeasi requested review from a team and mapsandapps and removed request for a team August 29, 2023 16:47

@mapsandapps mapsandapps 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.

It took me a while to figure out that none of the weird things I'm seeing with toolbars are actually related to the toolbar styles. It looks like this PR has all that's needed for the toolbar itself 👍🏻

@liamdebeasi
liamdebeasi merged commit e4cc457 into FW-4146 Aug 30, 2023
@liamdebeasi
liamdebeasi deleted the FW-4444 branch August 30, 2023 12:44
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