Skip to content
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

Synchronize cell "active" and "focused" states #2380

Closed
seancolsen opened this issue Jan 26, 2023 · 6 comments · Fixed by #2989
Closed

Synchronize cell "active" and "focused" states #2380

seancolsen opened this issue Jan 26, 2023 · 6 comments · Fixed by #2989
Labels
ready Ready for implementation restricted: maintainers Only maintainers can resolve this issue work: frontend Related to frontend code in the mathesar_ui directory
Milestone

Comments

@seancolsen
Copy link
Contributor

seancolsen commented Jan 26, 2023

Current behavior

  • Every cell in the sheet has the following state:

    • active or inactive

      No more than one cell can be active at a time, and we do our best (imperatively) to ensure that we always have one active cell.

    • selected or unselected

      Multiple cells can be selected within the sheet. We do our best (imperatively) to ensure that the active cell is also selected.

    • focused or unfocused

      No more than one cell can be focused at a time. But it's quite common to have zero cells focused -- and necessary so that the browser can focus other UI on the page. We have rather airtight logic to ensure that a focused cell is always active.

  • The user can interact with a table cell via the keyboard in several ways (e.g. Enter to enter edit mode, arrow keys to move the active cell, etc) -- but only when the cell is focused. If you click on some other UI like empty space above the table or a button somewhere else, the cell loses focus and those keyboard actions no longer have any effect even though the cell may still be active, but not the other way around.

  • We indicate that a cell is active by displaying a thick blue border if the cell allows editing (or displaying a grey border if the cell does not allow editing).

  • We have no indicator for the cell's focused state.

Problems with the current behavior

  • I don't care whether a cell is active -- I care whether a cell is focused. I can't identify any special behavior that an active-unfocused cell has over an inactive-unfocused cell. I can't do anything with it if it's not focused. Maybe I'm overlooking something here though.

  • The current behavior has the effect of making me unsure whether I can use the keyboard to interact with the cell, so I subconsciously end up using the mouse more.

Comparisons with other products

  • Google Sheets and LibreOffice Calc both behave very similarly to our current behavior. While that may be an argument for keeping it, I still don't like it.

  • AirTable almost entirely synchronizes their active cell with their focused cell, but they also discard the entire selection (and active cell) when the user focuses on other UI. I like the simplicity of this behavior, but it wouldn't be compatible with our table inspector design because we need to maintain a selection of cells while user is focusing on UI within the table inspector.

Desired behavior

  • Cell selection would work the same as it currently does, but we would eliminate the active/inactive state, keeping only focused/unfocused. When a cell is focused it gets a thick blue border (or grey if disabled). When a cell is unfocused it doesn't have the border. That's actually the behavior I built within the Record Selector, and I think it works well.

@kgodey @pavish @rajatvijay @ghislaineguerin I'm curious what you think about this idea. It seems like a change that would probably be easy to make.

@seancolsen seancolsen added status: draft work: frontend Related to frontend code in the mathesar_ui directory restricted: maintainers Only maintainers can resolve this issue labels Jan 26, 2023
@seancolsen seancolsen added this to the Backlog milestone Jan 26, 2023
@kgodey
Copy link
Contributor

kgodey commented Jan 26, 2023

I don't understand the purpose of an active/inactive state other than to indicate focus, so this proposal seems like an improvement to me.

@pavish
Copy link
Member

pavish commented Jan 26, 2023

A cell will lose focus when clicking elsewhere on the application, but we need it to be remain active/distinguished in some way while user is viewing the inspector (which is dependent on the current selected cell).

@seancolsen
Copy link
Contributor Author

@pavish the table inspector is based on the selected cells, not the active cell, right? I'm proposing we keep the cell selection the same, which seems to be compatible with the behavior of the table inspector.

@pavish
Copy link
Member

pavish commented Jan 26, 2023

Ah okay, I just noticed that.

The UX makes sense to me, but I think we'll need implementation details to be figured out and agreed upon before working on the changes.

While I think it would be a small change, I doubt this would be quick/easy. Both cell selection and cell focus are dependent on the active cell at the moment. We will have to change that logic to depend on focus.

Also, we might still need a way to keep track of the focused cell even when it loses focus, eg., when the user clicks on the cell, the input within it will get focused, but we'll still have to distinguish the cell. Also, when a cell has a dropdown, or a modal that opens and closes, we'll have to refocus on that same cell.

In addition to that, the current 'Cell' tab on data explorer is dependent on active cell.

These are the few things I can think on top of my head, anyone picking this issue up should go through the app and find out possible issues. I'd prefer an RFC for this.

@pavish
Copy link
Member

pavish commented Jan 26, 2023

The more I think about it, we do need the 'active' state. We cannot get rid of it.

I can't identify any special behavior that an active-unfocused cell has over an inactive-unfocused cell. I can't do anything with it if it's not focused. Maybe I'm overlooking something here though.

My previous comment should explain why we need to retain an active cell. Especially this part:

Also, we might still need a way to keep track of the focused cell even when it loses focus, eg., when the user clicks on the cell, the input within it will get focused, but we'll still have to distinguish the cell. Also, when a cell has a dropdown, or a modal that opens and closes, we'll have to refocus on that same cell.

The primary issue at hand is that the cell remains active even when user clicks on other parts of the app and that makes it unclear whether the active cell is focused. We need to figure out a way to clear the active cell when that happens. This will have the same UX expected from the issue description.

@seancolsen
Copy link
Contributor Author

@pavish

The primary issue at hand is that the cell remains active even when user clicks on other parts of the app and that makes it unclear whether the active cell is focused.

It sounds like we're in agreement about the main objective of this ticket. If there are good reasons for retaining the active/inactive state, then I'm fine with that and I think it can be considered an implementation detail within the work here. I'll keep this ticket restricted to team members since we expect some nuance to the implementation. Being that this is lower-priority at the moment, I think we can safely defer the bulk of the analysis and planning until the point where we pick this issue up for implementation.

That being said, I have a few more thoughts to relay while my mind is on the subject, but I'm not expecting us to reach a conclusion now about the implementation details.

  • when the user clicks on the cell, the input within it will get focused, but we'll still have to distinguish the cel

    We have a similar situation within the Record Selector when the child selector is open. We still need to distinguish the search input that triggered the child selector even though that input is no longer focused. For that purpose I have this code that maintains a dedicated state hasNestedSelectorOpen for that specific purpose. We could do something similar for a table cell where a cell gets styled in a similar manner if it is focused or if it is in edit mode.

  • Also, when a cell has a dropdown, or a modal that opens and closes, we'll have to refocus on that same cell

    We should be doing this anyway. If we're not re-focusing the cell, then we're leaving the user in a state where they can't continue keyboard interactions. We have Re-focus table cell after opening record selector on that cell #1892 because we're not currently re-focusing the cell after closing the record selector.

  • I notice that the "Cell" tab in the the Data Explorer currently operates only on the active cell. If we modified that UI to operate on the cell selection, then it could display other things when multiple cells are selected, as I'm proposing in Client-side aggregations from cell selection #2383.

@seancolsen seancolsen added ready Ready for implementation and removed status: draft labels Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready Ready for implementation restricted: maintainers Only maintainers can resolve this issue work: frontend Related to frontend code in the mathesar_ui directory
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants