Master named range adrm #7605
Open
Master named range adrm #7605
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commits improves the usability of the toolbar: - The composer is now in a new line to have more space for tools - Tools button have a bit more padding/margin - Added Insert Chart/Pivot/Function buttons in the toolbar Task: 5231006
Collaborator
|
|
With this commit, we now give the full proposal object as argument of `selectProposal` method of auto-complete providers, instead of only the text value of the proposal. This will be useful for the named range autocomplete, as we need to differentiate between a function and named range proposal. Task: 5380498
hokolomopo
force-pushed
the
master-named-range-adrm
branch
2 times, most recently
from
d5688ce to
36b71ca
Compare
hokolomopo
commented
Dec 5, 2025
| protected initialContent: string | undefined = ""; | ||
| private colorIndexByRange: { [xc: string]: number } = {}; | ||
| private autoComplete: Store<AutoCompleteStore> = new AutoCompleteStore(this.get); | ||
| private hasSelectedAProposal: boolean = false; |
Contributor
Author
There was a problem hiding this comment.
This kinda sucks ... but I don't have a better idea.
The problem is if we open a composer on a formula like =MyNamedRange. The autocomplete dropdown is open, and MyNamedRange is selected. If we press enter, provider.selectProposal will be called, do nothing, the dropdown will be closed ... and the promptly re-opened since nothing changed, so pressing enter will do absolutely nothing and we cannot close the composer.
And we don't want to hide the dropdown if this._currentContent === this.initialContent otherwise we don't have functions auto-complete either ...
hokolomopo
force-pushed
the
master-named-range-adrm
branch
from
36b71ca to
039a2a7
Compare
The `props.onChange` of the `TextInput` was called three times when
pressing enter:
1) once in the `onKeyDown` when we called `this.save()`
2) once in the `save` when we called `this.inputRef.el?.blur()`, which
triggered the `onBlur` event listener
3) once again in the `save` when we called `this.inputRef.el?.blur()`,
because HTML input elements trigger the `onChange` event when
losing focus.
This is very problematic when `props.onChange` dispatches a command
for example.
The issue was fixed by adding a `lastOnChangeValue` property that
stores the last value sent to `props.onChange`, and preventing another
call if the value did not change[1].
Another problem was the `save` had a `keepFocus` that was never used,
except accidentally when we gave the event in place of this argument.
[1] A better fix would have been to blur the input on `Enter`, and rely
on the `onChange` event triggered by the HTML input element when losing
focus. But this behavior, even if it's in the HTML spec, isn't
implemented in JSDOM. So all of our test would stop working.
Task: 5380498
hokolomopo
force-pushed
the
master-named-range-adrm
branch
from
039a2a7 to
5560880
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description:
description of this task, what is implemented and why it is implemented that way.
Task: TASK_ID
review checklist