Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Inline text formatting #1323
Inline text formatting #1323
Changes from 8 commits
536ac78
2e023e8
854510f
6fdac4c
e0506bf
323ec6c
6377f5a
48c765d
b0ba301
1656c0a
ad34f87
4a28da7
6346fa2
ebf8f26
04af4f5
e17c4f6
47df628
5dcaf4e
0f49ebd
e8b1f4e
9739dfb
41e6954
97a91cf
c4f4c18
057f33c
8097c3f
8685d11
3a80904
e2d2f23
4f58366
a0b3259
59134bc
ab85df5
37c9433
2f09bc6
7e2ed36
102c5af
4f88010
aa2fff6
44d7a77
b5a0c76
080251b
85edb58
e8f6266
f1dcb73
03aa64b
1b4f9de
f0dd127
d411b08
f1c8468
16dbf27
e295b37
870f2bd
e03baf3
0081a79
c5adfec
cd77cb9
7a28375
ba711d7
b63842a
b6468b0
595d4dd
13b6fda
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not all of the field updates are auto-submit, right? E.g.
letter-spacing
is not auto-submit. It's just a normal numeric input.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I now updates the text field as soon as a value is entered. But the new content is (as always) not submitted until the edit mode is exited.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain how this will go for letter-spacing? The sequence of input:
150
(initial value, indicating150%
)15
(the right-most character is deleted)1
(the right-most character is deleted)''
(the right-most character is deleted, this is not an empty string)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The input handling in the input component here is not optimal - it'll push its changes on every change, and then re-read the current value from the real input, resulting in empty value not being allowed, so it's un-intuitive to type some values (e.g. you cannot clear the text field). That's an optimization of the input component though, not the text formatter IMHO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Let's proceed and we'll need to see how we can follow up on this. For now this is one of few items that allows intermediate values. It might be best to do this update via the form's
usePresubmitHandler
, but can be done as a follow up.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just out of curiosity, would it have worked if we simply used here CSS notation. E.g.
BOLD-700
becomes a custom stylefont-weight:700
(orfontWeight:700
) instead? And so on. If possible, this could possibly simplify some of the downstream code.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. I'll think about it as I'll be refactoring this part of the codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not changed this part as of yet. It's still an open option though.