Skip to content

Update _floating-labels.scss#38023

Closed
jonnysp wants to merge 2 commits intotwbs:mainfrom
jonnysp:form-floating
Closed

Update _floating-labels.scss#38023
jonnysp wants to merge 2 commits intotwbs:mainfrom
jonnysp:form-floating

Conversation

@jonnysp
Copy link
Copy Markdown
Contributor

@jonnysp jonnysp commented Feb 8, 2023

add "pointer-events none" to pseudo-element
same as #37359

Description

Motivation & Context

Type of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would change existing functionality)

Checklist

  • I have read the contributing guidelines
  • My code follows the code style of the project (using npm run lint)
  • My change introduces changes to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

Live previews

Before
https://deploy-preview-38008--twbs-bootstrap.netlify.app/docs/5.3/forms/floating-labels/

After
https://deploy-preview-38023--twbs-bootstrap.netlify.app/docs/5.3/forms/floating-labels/

Related issues

add "pointer-events none" to pseudo-element
same as twbs#37359
@jonnysp jonnysp requested a review from a team as a code owner February 8, 2023 09:24
@julien-deramond
Copy link
Copy Markdown
Member

Thanks for this PR @jonnysp. Indeed sounds better with this modification.
However don't you think the rendering is a bit frustrating with the readonly floating label? It sounds like you can select the label's text but when you try you can't.

@jonnysp
Copy link
Copy Markdown
Contributor Author

jonnysp commented Feb 10, 2023

@julien-deramond
That is correct. The input cannot be focused when it is selected with the mouse, in the upper half area. on firefox, chrome and edge.

it is not for the read only label.

@jonnysp
Copy link
Copy Markdown
Contributor Author

jonnysp commented Feb 16, 2023

Recording 2023-02-16 at 14 47 02

@XhmikosR
Copy link
Copy Markdown
Member

@mdo @patrickhlauke regarding this.

We better be consistent here and make sure we still follow a11y best practices.

@patrickhlauke
Copy link
Copy Markdown
Member

the problem isn't present on the current https://getbootstrap.com/docs/5.3/forms/floating-labels/ ... so I'm wondering what changes were introduced that caused the problem in the first place.

there's also something funky happening with disabled form fields https://deploy-preview-38023--twbs-bootstrap.netlify.app/docs/5.3/forms/floating-labels/#disabled

image

@julien-deramond
Copy link
Copy Markdown
Member

julien-deramond commented Feb 18, 2023

FYI there was a conflict between several fixes on the floating labels between the v5.2.3 and v5.3.0-alpha1. Some use cases are broken in https://twbs-bootstrap.netlify.app/docs/5.3/forms/floating-labels/ (issues and PRs exist for that).
This PR is something else just to improve the clicking area but I admit it's a bit difficult to review without having the other fixes merged before.

IMHO the main issue that I can see here in this PR is about the read-only rendering where we can see the "selection" cursor when the labels are hovered even if we can't select their text (which is not the case in our main branch):

2023-02-18 07 26 33

The other use cases seemed to be enhanced when I looked at them the other day with an extended clickable area (possible to click on the upper area of the input to "enable" the input).

@patrickhlauke
Copy link
Copy Markdown
Member

FYI there was a conflict between several fixes on the floating labels between the v5.2.3 and v5.3.0-alpha1.

yeah sorry, what I meant is: i understand that, and am wondering if it's worth investigating how to resolve those conflicts at source, rather than trying to now just patch this one aspect with pointer-events on the floating label in this PR

@jonnysp jonnysp closed this Apr 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants