Change design of edit media modal in web UI #33516
Conversation
Gargron
force-pushed
the
feature-alt-text-modal
branch
3 times, most recently
from
76947c2 to
7162a18
Compare
Gargron
force-pushed
the
feature-alt-text-modal
branch
4 times, most recently
from
76f09c3 to
a88508a
Compare
|
A small feedback: the keyboard shortcut Other than that, I liked the changes. It's cleaner and minimalist, right to the point. |
|
Oh and this PR seems to break the CSS style of buttons (in the image, the Custom Emojis page).
|
Gargron
force-pushed
the
feature-alt-text-modal
branch
from
a88508a to
309aecd
Compare
|
I've restored |
Thank you! I applied the new code and noticed that it's possible to describe the image without a limit of characters, but when the user clicks the Done button, a message Also, a nitpick suggestion: to focus the text field when the modal opens, so it's ready to write the description. |
|
Another thing I just noticed: in the previous design, when the user type something and by mistake press Esc or something equivalent, there's a prompt about discarding everything or to cancel and go back to typing the alt-text. With the new design, when the user press cancel in this scenario and go back to typing, everything written is gone. |
Gargron
force-pushed
the
feature-alt-text-modal
branch
3 times, most recently
from
84c1be1 to
4444993
Compare
|
All addressed. |
|
I have yet to investigate why this happens, and I have not reviewed the code yet, but the following fails:
When doing this, the description from step 2, not step 3, is persisted. |
Gargron
force-pushed
the
feature-alt-text-modal
branch
from
4444993 to
a32193b
Compare
When clicking the Edit or ALT buttons, the text field to type the alt-text is not focused. But if this nitpick is not planed, no problem. |
There was a problem hiding this comment.
This is a fairly big change that deals with a redesign, multiple refactors, a dependency update and a few unrelated CSS changes. I would have appreciated if it were split in easier-to-review, more logical commits. As it stands, I have not been able to do an in-depth review, only skim over the code and test the functionality.
Also, it might be useful to have system tests for the different edit flows, as they are complex and were broken at multiple points in this PR.
Includes an upgrade to Tesseract.js which according to the docs should be about 10x faster than the previous version we used.
Fixes MAS-258