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

Change the data structure to store selections in the selection store #1732

Open
rajatvijay opened this issue Sep 26, 2022 · 2 comments · May be fixed by #3037
Open

Change the data structure to store selections in the selection store #1732

rajatvijay opened this issue Sep 26, 2022 · 2 comments · May be fixed by #3037
Assignees
Labels
ready Ready for implementation restricted: maintainers Only maintainers can resolve this issue type: enhancement New feature or request work: frontend Related to frontend code in the mathesar_ui directory
Milestone

Comments

@rajatvijay
Copy link
Contributor

rajatvijay commented Sep 26, 2022

Problem

The current structure is only capable to store the selected cells.
selectedCells: WritableSet<string>;
This is not usable when the table has no rows hence no cells but the user should still be able to select columns and edit their properties as mentioned in #1684

The rationale behind current implementation

The current implementation is based on the fact that when a cell is selected, a row and a column are also selected by default. Row selection can be described as selecting all the cells in a particular row.
Column selection can be described as selecting all the cells in a particular column.

Proposed solution

@pavish proposed the following design:

The selection store to utilize a different structure instead of just the identifier string array, something like:

{
   type: 'cell' | 'column' | 'row',
   identifiers: string[],
}

The identifiers here would depend on the type of selection.

@rajatvijay rajatvijay self-assigned this Sep 26, 2022
@rajatvijay rajatvijay added status: draft work: frontend Related to frontend code in the mathesar_ui directory and removed status: triage labels Sep 26, 2022
@rajatvijay rajatvijay added this to the 03. First Release milestone Sep 26, 2022
@rajatvijay rajatvijay added the restricted: maintainers Only maintainers can resolve this issue label Sep 26, 2022
@seancolsen
Copy link
Contributor

I just merged #1733 which moves us forward with the "current implementation", but I do think it's worth considering ways to improve this code.

Although I initially expressed support for the current implementation, I think @pavish has a good idea (if I'm understanding it correctly). To flesh this idea out a bit more I'd propose the following:

In src/stores/table-data/selection.ts...

  1. Rename the Selection class to SelectionStore.

  2. Use the following:

    interface SelectionOfCells {
      basis: 'cells';
      cellIds: ImmutableSet<string>;
    }
    interface SelectionOfColumns {
      basis: 'columns';
      columnIds: ImmutableSet<number>;
    }
    
    type Selection = SelectionOfCells | SelectionOfColumns;
    
    class SelectionStore {
      // ...
    
      selection: Writable<Selection>;
    
      // ...
    }

This way, the selection is either a selection of cells or a selection of columns, but never both. The SelectionOfColumns type would only be used when the table is empty, similar to how we're currently doing it. I don't currently see a need for tracking a selection of rows, but if such a need arose in the future, we could modify the Selection type accordingly.

@github-actions
Copy link

This issue has not been updated in 90 days and is being marked as stale.

@github-actions github-actions bot added the stale label Apr 16, 2023
@rajatvijay rajatvijay removed the stale label Apr 17, 2023
@seancolsen seancolsen self-assigned this Jul 12, 2023
@seancolsen seancolsen linked a pull request Jul 13, 2023 that will close this issue
7 tasks
@seancolsen seancolsen modified the milestones: Backlog, v0.1.4 Aug 17, 2023
@seancolsen seancolsen modified the milestones: v0.1.4, Backlog Oct 2, 2023
@seancolsen seancolsen added ready Ready for implementation and removed status: started labels Dec 5, 2023
@kgodey kgodey modified the milestones: Backlog, v0.1.7 Apr 3, 2024
@ghislaineguerin ghislaineguerin removed this from the v0.1.7 milestone Apr 15, 2024
@ghislaineguerin ghislaineguerin added this to the High priority milestone Apr 15, 2024
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 type: enhancement New feature or request 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.

4 participants