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

LabeledFieldView with a value renders label incorrectly when disabled #10431

Closed
oleq opened this issue Aug 26, 2021 · 2 comments · Fixed by #10713
Closed

LabeledFieldView with a value renders label incorrectly when disabled #10431

oleq opened this issue Aug 26, 2021 · 2 comments · Fixed by #10713
Assignees
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:find-and-replace package:theme-lark package:ui squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@oleq
Copy link
Member

oleq commented Aug 26, 2021

📝 Provide detailed reproduction steps (if any)

A generalization of #10429.

We didn't notice this before because the only place where LabeledFieldView was getting disabled was the table properties form and we reset the value there automatically when the field gets disabled. Now we have this use-case in find and replace and probably more will come

❓ Possible solution

Most likely this will require only changes in CSS.


If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@Reinmar
Copy link
Member

Reinmar commented Sep 23, 2021

Refinement notes:

  • Tricky to reproduce
  • Sometimes happens... sometimes not
  • Not clear what may be wrong here (focus or emptiness detection? browser bug?)
  • Estimate: 1pt as the top ROI margin

@oleq oleq self-assigned this Oct 18, 2021
@oleq
Copy link
Member Author

oleq commented Oct 18, 2021

I couldn't reproduce it in the F&R UI but it is still in the table props form:

What's going on here:

  1. The input CSS .ck-input-text_empty class that controls the look and the location of the label is set upon DOM #change.
  2. The color input is focused by the user (empty = true, focused = true, readonly = false).
  3. The user is typing.
    1. There was no DOM #change event yet (empty = true, focused = true, readonly = false).
  4. They click the dropdown and change the border type. This:
    1. Does not blur the color input
      1. Still no DOM #change event (empty = true, focused = true, read-only = false)
    2. Sets readonly = true on the color input DOM element, which changes the way it looks (empty = true, focused = true, read-only = true)
  5. The CSS of the field never assumed that it is possible to have an "empty" (at least by CSS), read-only, and focused (at the same time) input with some uncommitted value rendered (still, no #change...). If it's empty, there should be nothing there so it is safe to overlay the label.

The problem is the .ck-input-text_empty class logic. It is updated only on blur/enter. Which is too late; it stays out of sync.

dawidurbanski added a commit that referenced this issue Oct 19, 2021
…clash

Fix (ui): InputTextView should update its #isEmpty property on every #input instead of #change to stay in sync. Closes #10431.
@AnnaTomanek AnnaTomanek added this to the iteration 48 milestone Oct 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:find-and-replace package:theme-lark package:ui squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants