-
-
Notifications
You must be signed in to change notification settings - Fork 338
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
fix: cell loses focus when clicking on its outline #3185
fix: cell loses focus when clicking on its outline #3185
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.
This change is not acceptable because it modifies the visual design of the active cell indicator.
Before | After |
---|---|
Before your change, the blue outline lay outside the cell rectangle. After your change, the blue outline lies inside the cell rectangle.
We want to preserve the visual design we already have.
Hi @seancolsen, Based on my understanding, there appear to be two issues: (1) The first issue arises when a user clicks between the edge of the cell and the box-shadow, causing the cell to lose focus. This behavior is not a bug but rather the default behavior of the browser, and it is consistent across various websites such as GitHub and YouTube. bug reproducegithubyoutube(2) The second issue occurs when a user clicks at the edge of the box-shadow, causing focus to shift to another cell. bug reproduceIf my interpretation is correct, it seems that the second issue is the one we should consider a bug or unexpected behavior. If we consider the second issue as the primary bug, here is a proposed solution: We can apply two types of box shadows, one with a 1px inset shadow and the other with a 1px outset shadow. This approach can help prevent the occurrence of this bug. With the addition of both inset and outset shadows, the behavior changes in a way that prevents focus from shifting to another cell. This is because the box shadow does not extend beyond the border. behavior after inset and outset shadowscode changes for 2nd issue
@seancolsen, I'd like to hear your thoughts on whether we should address the second issue. If we prioritize the first issue, please share any potential solutions you have in mind. |
@manthan-jsharma No, this issue is actually not about either Let me take a step back and explain why this is important... When a cell element has focus, many different keyboard shortcuts are active. For example, I can use arrow keys to move the active cell. Our UX design is such that, when the cell loses focus, those keyboard shortcuts are no longer active. This UX design has some drawbacks but it somewhat important for usability and accessibility throughout the rest of the app (e.g. within the UI of the table inspector). This UX design is also consistent with Google Sheets, for example. The drawback is that it becomes quite important to ensure that the user doesn't inadvertently steal focus away from the cell. If they do, then we could be interrupting their work, requiring more mouse usage and slowing things down. Due to these requirements, it should be the case the clicking anywhere within the sheet is a safe operation focus-wise. That is, there should be clickable areas within the sheet that steal focus from cells. This issue describes a very thin area between cells which steals focus from cells. If I click click on a horizontal line between two cells, which cell should be selected? The top one? The bottom one? It doesn't matter! The important thing is that some cell should be selected and focused, otherwise we have an area within the sheet that steals focus from cells. Does this make sense? |
Hi @seancolsen i got the issue now, whenever i click on border line of unselected cell, it doesn't get focus.(border is that thin line) As a solution, I suggest adding an "onclick" event to the cell border so that it can get focus. Could you please confirm if we can proceed with this solution? |
@manthan-jsharma I'd rather not add more event listeners to handle this. We currently have several event listeners implemented per cell, and in the long term we'd actually like to remove those for performance reasons. I would imagine that we'd be able to fix this issue with the proper markup and CSS. The active cell border should be visual only — no behavior, so no JS. |
@manthan-jsharma I'll leave this PR open for another couple weeks and then close it if we haven't heard from you. |
As you have already mentioned the specific direction you want this change to be proceeded so that it does not cause any redundancy on the established designs and code structures associated to it, i have thought of working it out with a pseudo element layer above the shadow box or something along the line(i will be extrapolating it further) , that would potentially pertain the click event to be always at call whenever its needed. Consequently, if a click occurs on the specific edges we've introduced, the event and the associated focus will be maintained. |
@manthan-jsharma from your last comment I'm not sure exactly what you're proposing, but I'll be happy to review your next iteration of it once I see the code. |
@manthan-jsharma are you still working on this? I'll close it if we don't see any continued work here in a couple weeks. |
Fixes #2987
Technical details
rectifying the issue with fixating upon the blue and grey box shadow by making it inset
Screenshots
Checklist
Update index.md
).develop
branch of the repositoryvisible errors.
Developer Certificate of Origin
Developer Certificate of Origin