-
Notifications
You must be signed in to change notification settings - Fork 66
[IMP] dv: autofocus input #7252
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
Conversation
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.
👋
| false | ||
| )}`; | ||
| const message = () => { | ||
| if (this.isNot && pass) { |
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 we do something generic ? All of the other custom matchers are also wrong ...
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.
let's do this in another task
| () => { | ||
| if (this.props.focused) { | ||
| this.inputRef.el!.focus(); | ||
| if (this.props.focused && this.inputRef.el) { |
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 the input ref really be undefiend at useEffect ? Or is it just to avoid ! ?
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.
yes, if the input isn't displayed because it's a composer instead
|
|
||
| onCriterionTypeChanged(type: DataValidationCriterionType) { | ||
| this.state.rule.criterion.type = type; | ||
| this.state.isTypeUpdated = true; |
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.
we should probably have the same behaviour for CFs and in the data filter menu, no ? To confirm with frgi, but I don't see why implement it in DVs but not CFs
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.
+1
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.
It's done 😉
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'm blind
b2a14ee to
2fe397c
Compare
| false | ||
| )}`; | ||
| const message = () => { | ||
| if (this.isNot && pass) { |
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.
let's do this in another task
|
|
||
| onCriterionTypeChanged(type: DataValidationCriterionType) { | ||
| this.state.rule.criterion.type = type; | ||
| this.state.isTypeUpdated = true; |
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.
+1
When switching a data validation to "Value In Range", automatically focus the range input to allow selecting a range right away without an additional click to focus the input. We do the same for all validation types that have an associated input. Task: 4862354
Task: 4862354
Task: 4862354
2fe397c to
e011499
Compare
| t-key="state.rules.cellIs.operator" | ||
| criterion="genericCriterion" | ||
| onCriterionChanged.bind="onRuleValuesChanged" | ||
| autofocus="this.state.hasEditedCf" |
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.
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.
from a functional POV, probably yes it makes sense. From a technical POV, is it worth it ? I don't know...
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.
let's keep it as it is for now then
|
robodoo r+ rebase-ff |
|
Merge method set to rebase and fast-forward. |
Part-of: #7252 Signed-off-by: Rémi Rahir (rar) <[email protected]>
When switching a data validation to "Value In Range", automatically focus the range input to allow selecting a range right away without an additional click to focus the input. We do the same for all validation types that have an associated input. Task: 4862354 Part-of: #7252 Signed-off-by: Rémi Rahir (rar) <[email protected]>
Task: 4862354 Part-of: #7252 Signed-off-by: Rémi Rahir (rar) <[email protected]>
closes #7252 Task: 4862354 Signed-off-by: Rémi Rahir (rar) <[email protected]>



Description:
description of this task, what is implemented and why it is implemented that way.
Task: 4862354
review checklist