Skip to content

fix(input, textarea): input does not block floating label - #27870

Merged
liamdebeasi merged 10 commits into
mainfrom
FW-4833
Aug 1, 2023
Merged

fix(input, textarea): input does not block floating label#27870
liamdebeasi merged 10 commits into
mainfrom
FW-4833

Conversation

@liamdebeasi

@liamdebeasi liamdebeasi commented Jul 27, 2023

Copy link
Copy Markdown
Contributor

Issue number: resolves #27812


What is the current behavior?

We currently have CSS to ensure the floating/stacked label is not obscured by the input/textarea when using the outline styles. However, it was discovered that this scenario can happen with any floating/stacked label not just when the input/textarea is using the outline style.

What is the new behavior?

  • Floating/stacked labels appear on top of the input/textarea regardless of fill mode.

Does this introduce a breaking change?

  • Yes
  • No

Other information

Dev build: 7.2.1-dev.11690464203.1b2b4419

@github-actions github-actions Bot added the package: core @ionic/core package label Jul 27, 2023
@liamdebeasi liamdebeasi changed the title fix(input, textarea): input does not block label fix(input, textarea): input does not block floating label Jul 27, 2023
@liamdebeasi
liamdebeasi marked this pull request as ready for review July 27, 2023 13:51
@liamdebeasi
liamdebeasi requested review from a team and mapsandapps and removed request for a team July 27, 2023 13:51
@liamdebeasi

liamdebeasi commented Jul 27, 2023

Copy link
Copy Markdown
Contributor Author

The textarea screenshot that changed looks to be due to a browser quirk in Firefox:

image

Changing the z-index resulted in a 1px diff at the edge of the input. I think this change is fine given that it appears to be a browser rendering quirk.

edit: Zoomed in so it's easier to see:

image

@github-actions github-actions Bot added the package: angular @ionic/angular package label Jul 27, 2023
@liamdebeasi
liamdebeasi enabled auto-merge July 27, 2023 14:22
Comment thread packages/angular/test/README.md Outdated

If you need to add E2E tests that are only run on a specific version of the JS Framework, replicate the `VersionTest` component on each partial application. This ensures that tests for framework version X do not get run for framework version Y.

### Testing Lazy Loaded Ionic Components

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 change should be reverted. It probably just got tracked between switching branches.

@liamdebeasi liamdebeasi Jul 27, 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.

Well that explains why the docs didn't show up the first time I pushed to #27867 🤦

Fixed in 0713c14

This reverts commit b4bad45.
@github-actions github-actions Bot removed the package: angular @ionic/angular package label Jul 27, 2023
@liamdebeasi
liamdebeasi requested a review from sean-perkins July 27, 2023 17:46

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

Looks good. I confirmed that the fix works (and I saw the bug submitter confirmed as well), and it looks like everything else works like before. 👍🏻

});
});
test.describe(title('input: floating/stacked label layering'), () => {
test('label should not be covered by text field', async ({ page }) => {

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.

Let's add a testInfo.annotations to track the reason behind this test.

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.

Fixed in 9ffc1d9

});
});
test.describe(title('textarea: floating/stacked label layering'), () => {
test('label should not be covered by text field', async ({ page }) => {

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.

Let's add a testInfo.annotations to track the reason behind this test.

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.

Fixed in 9ffc1d9

@liamdebeasi
liamdebeasi requested a review from thetaPC July 28, 2023 13:56

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

LGTM

@liamdebeasi
liamdebeasi added this pull request to the merge queue Aug 1, 2023
Merged via the queue into main with commit f14c440 Aug 1, 2023
@liamdebeasi
liamdebeasi deleted the FW-4833 branch August 1, 2023 16:07
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: browser autofill background color overlaps input label

5 participants