vaadin
Spreadsheet focuses the sheet rather than the component when displaying a cell edit component
Description
When a cell with a custom editor is clicked, the custom editor is displayed instead of (or on top of, hiding) the cell static contents. However, after displaying the editor component, the SheetWidget client-side code sets the focus to the sheet, not the editor.
This line is from the initial code commit 10 years ago, per Git-Blame. To my mind that means there is no institutional knowledge about why that focus choice is there :) I don't remember my 10 year old code, at least.
This causes custom edit component use to require multiple clicks, and even breaks keyboard navigation, as attempting to edit a cell suddenly jumps focus back to the sheet, requiring navigating to the cell once again before the component can be used.
Further, I noticed this focus shift also interferes with a ComboBox used as an edit component with autoselect=true such that once the user finally activates the component the filter contents are no longer selected.
Expected outcome
Clicking a Spreadsheet cell with a custom editor should display and focus the editor.
Minimal reproducible example
define a SpreadsheetComponentFactory subclass and add it to a Spreadsheet component. Implement only getCustomEditorForCell() and return a simple component, in my case a ComboBox, for all cells or a specific cell.
In the browser, click that cell, and note where the focus ends up after one cell click.
Steps to reproduce
In the browser, click that cell, and note where the focus ends up after one cell click when the cell has a custom edit component.
Environment
Vaadin version(s): 24.4.8 OS: N/A
Browsers
Issue is not browser related
Hi @WoozyG, It seems to me that the described behavior is pretty much in line with how the default editor behaves: a single click on a cell only focuses the cell, and does not place focus in the editor. If a single-click did focus the editor, it seems to me that that would make it rather difficult to focus the cell itself, when that is desired.
Perhaps the problem is that the editor component being revealed on click causes a confusing UX that raises expectations of it also getting focused? (Compared to default cells which don't change in appearance when clicked, that is.)
@rolfsmeds excellent point. Our use case doesn't include needing to select a cell with an editor component, those cells always want the editor focused on click, vs. cells with plain text or numbers, for example, which in our use case are locked (non-editable).
Maybe the real question is can we have a way to tell the client implementation what to do when displaying an edit component?
If it isn't visible on first click, not only does it take two clicks, you don't know it needs a second click, as there is no indication it would do anything, especially if other cells are locked and not editable.
Perhaps I could also look at always displaying the component, as cell contents, rather than just as an edit component. We chose not to do that originally (7 years ago) as the guidance was to not show too many components so it didn't bog down the UI. However for us, we only have a few per sheet, and use them as dynamic filter controls to let users change what data is displayed in tables and graphs.
Simplest from a code change on my end viewpoint is an API enhancement to allow the backend code that generates/returns the edit component to specify if it receives focus immediately or if the containing cell does.
More involved would be rewriting things to display the components always, so a cell click could trigger an event that focused the component, although even in that case it might be nice to have a way to tell the UI that's the behavior we want, to avoid the delay of a round-trip.
It seems to me that two behaviors would be reasonable options:
- A) Custom editor is displayed only when you double-click/Enter/F2 the cell to edit it
- B) Custom editor is always displayed, and a single-click focuses or otherwise activates it (e.g. toggles a checkbox)
Option A would be consistent with default cells/editors, while option B would be consistent with how fields and other components work outside of the Spreadsheet.
The current behavior, where custom editors become visible on cell focus, and focused (or otherwise activated) with a single-click seems like a strange compromise between the two that is consistent with absolutely nothing. To make things worse, double-clicking a cell with a custom editor that isn't already focused, or pressing Enter/F2 on a focused custom-editor cell, do nothing.
The problem is of course that changing the default to A or B is a behavior altering change, but personally I think it's small enough that it would be ok to introduce it in a minor version, especially considering that the current behavior is certainly a design flaw if not an outright bug.
Out of the two behaviors, perhaps option B would run the lesser risk of confusing/angering existing users used to the current behavior?
In any case, this would ideally then be together with
- an API for switching an editor the other behavior
- fixing Enter/F2 editor activation on custom-editor cells
I like option B for our purposes, don't know what other folks might use custom editors for, but for us, they are not for table data edits but for formula control parameters, so only a few per sheet, and having them always visible seems like a good improvement in usability. Plus these would less often be cells folks would want to select in ranges, and if they did, they could start from a different cell and drag.
Ok, we've decided to make this change, it's in our backlog, we deem the change in behavior small enough to be acceptable in a minor version, but as it's more of refactor+enhancement, it's not considered a bug.
Great, thanks for the update. I'll stop toying with it and work on other final bugs in my alpha :)
My first attempt to prototype this was to add a check for null after the call getCustomComponentForCell here to then call getCustomEditorForCell, so it would be a "fallback".
https://github.com/vaadin/flow-components/blob/e02d28a7a63337e98fe48758935b0516d821286e/vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow/src/main/java/com/vaadin/flow/component/spreadsheet/Spreadsheet.java#L4557-L4559
I also had to do some other changes in the client code, but the editor was being displayed. However there were some nasty behavior that needs checking where, after interacting with a ComboBox, the element was removed and focusing on the cell again wouldn't make it show again. I published the proto on this the proto/spreadsheet/custom-editor branch.
Any update to this? Just got back to the internal issue referencing it. Hoping to FINALLY hit alpha next month, with real customer power users taking it for a spin.
No movement yet? This will be a blocker for allowing customers to see our new version.
Dear @WoozyG,
We've completed the initial prototyping but haven't been able to proceed further yet. We'll try to include it in the scope in the near future and will notify you of any progress.
This was incorrectly closed - that PR is for Accordion, not Spreadsheet, it tagged the wrong issue.
For background, V8 had similar behavior (not exactly the same, but still problematic from a usability perspective). However, in V8 I was able to subclass Spreadsheet and the GWT Java client classes to correct/improve the behavior. I can't do that now in Flow, as the GWT compiled code is part of the component. So for me, this is a loss of flexibility/customizability due to the technology shift from GWT to Web Components. Thus needing to file an issue rather than just coding up a solution that works in our use case.
Hi @WoozyG,
Sorry for the lack of feedback. I am working on this feature, and I've been making some adjustments and getting the code in better shape before submitting it to review. I am planning to create a couple of PRs, one for adding the ability to set whether custom editors should be always visible and another to fix the keyboard/mouse interaction with custom editors that is broken at the moment.
I am hoping to make them ready to review soon.
This ticket/PR has been released with Vaadin 24.8.0.beta1 and is also targeting the upcoming stable 24.8.0 version.
Do any tests of this issue include multiple cell editors at once? What about navigation between them? We are seen a crazy number of server round-trips, and apparent duplicate events when using the arrow keys or tab/shift+tab to navigate between two cells with editors. Wondering if it is in the framework or the complexity of our edit components.
The tests have a few cells with editors in the same sheet. It's possible that we're not checking the number of events triggered while interacting with them, so we would need to investigate this further to understand the reason.
Also, I realized when the dogs woke me up overnight that always displaying the editor components now violates the API documentation, which says custom editors allow for component re-use. Probably the code and tests need to check for that, and only allow re-use when the new flag is off, and the JavaDoc needs updating to reflect the more nuanced behavior.
One more oddity - now, when a cell has a custom editor component, say a ComboBox or something, the cell starts by displaying the plain text value, formatted with the cell formatting. Navigating into the cell displays the custom editor instead, as expected. But navigating out of the cell leaves the editor displayed, instead of showing the potentially updated cell value with formatting. This is with the default behavior of the new "showCustomEditorOnFocus" flag, which is false.
Further, setting it to "true" only displays the editor component when the cell has focus, as expected, but when leaving the cell, the cell displays as blank! Navigating to a different tab and back shows the cell value as unchanged, so the blanks were not saved to the POI object, but are display anomalies.
The above issue with cell contents disappearing may be related to #7507 which we already prioritized. I strongly suspect that's the case, as only sheets with freeze panes seem to show that issue for me.
I may have worked around or even eliminated the event issue by adjusting my code to be more nuanced on when it does expensive actions. Might be worth noting in the docs that custom editor components that display on cell selection or typing should not require expensive actions on every display, or client UI timing issues can arise that the framework can't help with.
Glad you were able to work around the event issue. Would you kindly create new tickets about the issues you have found so far with the feature, so that we can track them more easily? Adding some reproducible code, whenever possible.