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

AttributeTable : non-editable cells are now selectable #131

Merged
merged 2 commits into from
Sep 4, 2024

Conversation

mukoki
Copy link
Contributor

@mukoki mukoki commented Aug 11, 2024

Original request
User request was to be able to copy/paste attribute value from AttributeTable, in the case of not editable layers.

Use TableCellEditor instead of TableCellRenderer
I could not make cell values selectable from the TableCellRenderer. Hence, to make selection/copy/paste possible TableCellEditor is now used in both cases of editable and non-editable layers.
To prevent edit operation in the non-editable layer case, the editable component is finalized (made editable or not) during the preparation phase of the component (prepareEditor).

Other improvements
Small improvements in the cell rendering (do not truncate letters anymore in edition mode) and in the popupmenu used to manage null values.

// layer.getFeatureCollectionWrapper().getFeatureSchema();
// if (schema.isAttributeReadOnly(schema.getAttributeIndex(getColumn(
// columnIndex).getName())))
// return false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does attribute is readonly not work? why did you comment this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works, but to make selection possible, I now always use a cellEditor. Once cellEditor is activated, we still can disable edition for readonly-attributes (done in prepareEditor method). I did not find an easier path as cellRenderer is not selectable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let me rephrase. why is this still there but commented so it does not do anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will remove the comments

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i sometimes leave commented code but prefix it with a comment that it is some reference of how to do something alternatively or such

but if it really is just old/redundant please remove :)

}
// if (!layer.isEditable()) {
// return false;
// }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as below

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about this?

@edeso
Copy link
Member

edeso commented Sep 3, 2024

tried the changeset but fail to see the difference in behaviour. could you please explain in more detail what this solves? with and without this patch i can select rows in attribute table and copy the contents, regardless if the layer is editable or not.

@edeso edeso force-pushed the make_attribute_table_cell_selectable branch from b1cbd61 to 37d26c3 Compare September 3, 2024 11:20
@jratike80
Copy link
Member

Right, you can select rows. But you cannot select contents from individual cells.
image

@mukoki
Copy link
Contributor Author

mukoki commented Sep 3, 2024

Yes, Jukka got it. Before the change, if you wanted to copy paste a single value from the attribute table of a non editable layer to your favorite text editor, you could not. Now, you can.

@edeso
Copy link
Member

edeso commented Sep 3, 2024

  1. hmm, this (let's call it) sub-selection dotted-border of one cell is hardly visible on my screen. that should be clearer.
  2. how do you copy one cell only? Ctrl+C copy the whole row for me. do you mean doubleclicking, selecting and the copy?
  3. while we're at it. selection in attribute table is inferior to what one is used to from spreadsheet software. there seems to be no way to select columns or multiple cells, is that right?

@jratike80
Copy link
Member

  1. Exactly, double-click and then paint or use Ctrl-A for selecting the whole content, then Ctrl-C
image
  1. Right, it is not possible to select multiple cells.

@edeso
Copy link
Member

edeso commented Sep 3, 2024

  1. Exactly, double-click and then paint or use Ctrl-A for selecting the whole content, then Ctrl-C
    image

ok, then some sub-selection decoration as mentioned in 1. is clearly not needed. wondering how intuitive this is tough. a possibility to select single cells and rows and columns by select the table headers would be more in line what a user would expect or?

@jratike80
Copy link
Member

ok, then some sub-selection decoration as mentioned in 1. is clearly not needed. wondering how intuitive this is tough. a possibility to select single cells and rows and columns by select the table headers would be more in line what a user would expect or?

It depends on what other software user is most familiar with. Excel users may expect that it is possible to select a group of cell by dragging a box and that click on the column header would select the whole column. On the other hand, that clicking the header sorts rows according to that column is also common.

In QGIS when in read-only state:

  • It is possible to select the contents of one, and only one whole cell with Ctrl-C or through a right-click menu. Sub-selection is not possible.
image
  • It is possible to select one or many full rows, including the geometry. Rows do not need to be consecutive.
  • It is possible to select everything with Ctrl-A
  • Click on the column header sorts data.

The QGIS behavior is close to the new suggested OJ selection.
Some observations:

  • In QGIS and OJ there is no way to clear the selection of attribute data with keyboard. QGIS has a button for that purpose. OJ might have, too. However, in OJ selecting rows from the attribute window does not select features on the map by the same like QGIS does, but OJ user must press the "Select in Project Window" button first.
image
  • In QGIS user must click on the row number for selecting a row. The row number is always visible so user does not need to scroll horizontally. OJ selects a row when user clicks anywhere over the row.

For my mind the OJ way is slightly better. Ctrl-C in QGIS when is this state copies "Sottunga"
image
but in this state the whole row gets copied
image
and also in this state the row 4 gets copied. For copying "Kerava" user needs to use the right-click menu.
image

@mukoki
Copy link
Contributor Author

mukoki commented Sep 3, 2024

Thank you for the complete review Jukka.
You may know that OpenJUMP has a hidden option (in configuration panel) to synchronize attribute table selection with view selection.
It can be a little disturbing though, as it does not clear selected features of another layer. Does not work well with InfoFrame.

@edeso
Copy link
Member

edeso commented Sep 4, 2024

@jratike80 sounds like you are pretty satisfied with the status quo?

wrt. this patch i approve. @mukoki feel free to squash/merge it :)

@jratike80
Copy link
Member

Yes, I am satisfied with this PR.

@edeso
Copy link
Member

edeso commented Sep 4, 2024

@jratike80 nope, i mean with the selection capabilities in general as of now in the Atrribut Table. or do you miss something crucial?

@jratike80
Copy link
Member

Nothing crusial, but selecting row(s) or all data selects also geometries in this form:
com.vividsolutions.jump.feature.FlexibleFeature@3b473917
I have never understood what I could do with such FlexibleFeatures. They cannot be even pasted back to OJ, or do they? For me they are something I do not want and what I need to delete afterwards. In some use cases I might have use for a WKT geometry.

A possibility to select blocks of a few rows and columns would be nice sometimes as all spreadsheet users know, but I do not know how the connection with Project windows would work.
image

@mukoki mukoki merged commit 61f47b5 into main Sep 4, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants