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

fix(searchbar): cancel icon aligns with back button #28478

Merged
merged 7 commits into from
Nov 8, 2023
Merged

Conversation

liamdebeasi
Copy link
Contributor

@liamdebeasi liamdebeasi commented Nov 6, 2023

Issue number: resolves #28468


What is the current behavior?

The cancel button in the searchbar used to align with the back button, but that behavior gradually regressed between Ionic v4 and Ionic v7.

What is the new behavior?

  • Adjusted the padding on the searchbar back button
  • Reduced the size of the searchbar back button. It was currently set to 25.6px whereas the ion-back-button icon was 24px. This caused the icons to never align even with 9px. (This should cause a few additional diffs)
  • Added a screenshot test
v4 v5 v6 v7 branch
Screenshot 2023-11-06 at 5 41 16 PM image image image image

Note that in v4 the alignment was slightly off. It was fixed in v5, but then broke in v6 and remained unchanged in v7 (until now).

Does this introduce a breaking change?

  • Yes
  • No

Other information

Dev build: 7.5.4-dev.11699310489.151b2717

@github-actions github-actions bot added the package: core @ionic/core package label Nov 6, 2023
@liamdebeasi liamdebeasi changed the title Fw 5541 fix(searchbar): cancel icon aligns with back button Nov 6, 2023
@liamdebeasi liamdebeasi marked this pull request as ready for review November 6, 2023 22:59
Copy link
Member

@brandyscarney brandyscarney left a comment

Choose a reason for hiding this comment

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

I took a look at this and I had different results. Using the images in the description, v4 was also slightly misaligned:

version-comparison

I also took a closer look at the padding used in this PR (8px) because it looked off just barely, and found that using 9px seems to get us closer in alignment:

different-pixel-padding

Let me know if you are seeing something different or if you need a higher res of my last image.

core/src/components/searchbar/test/basic/searchbar.e2e.ts Outdated Show resolved Hide resolved
Copy link
Member

@brandyscarney brandyscarney left a comment

Choose a reason for hiding this comment

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

Looks good! 👍

@liamdebeasi liamdebeasi added this pull request to the merge queue Nov 8, 2023
Merged via the queue into main with commit c053fd9 Nov 8, 2023
45 checks passed
@liamdebeasi liamdebeasi deleted the FW-5541 branch November 8, 2023 17:27
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.

bug: md, the back and search buttons are not aligned
3 participants