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

feature request: right click to delete a row or a column #1283

Closed
WiliTest opened this issue Jan 4, 2018 · 20 comments
Closed

feature request: right click to delete a row or a column #1283

WiliTest opened this issue Jan 4, 2018 · 20 comments
Assignees

Comments

@WiliTest
Copy link

@WiliTest WiliTest commented Jan 4, 2018

Details for the issue

I struggle to find how to delete a row*. It would be intuitive to be able to right click on the row number (in the tab "browse data"), or on the field headings and have an option "delete the column" or "delete the row".

Thanks (in all case!)

  • for those who don't know how to delete the column: database structure tab, click on the table, the "modify table" button will be active. Click on it, select the column you need to delete, click on remove field.

I'm opening this issue because:

  • DB4S is crashing
  • DB4S has a bug
  • DB4S needs a feature
  • DB4S has another problem

I'm using DB4S on:

  • Windows: ( version: 10 )
  • Linux: ( distro: ___ )
  • Mac OS: ( version: ___ )
  • Other: ___

I'm using DB4S version:

  • 3.10.1
  • 3.10.0
  • 3.9.1
  • Other: ___

I have also:

@justinclift
Copy link
Member

@justinclift justinclift commented Jan 4, 2018

Seems reasonable. 😄

mgrojo added a commit that referenced this issue Jan 4, 2018
The text is got from the "Delete record" button, so the text is updated to
"Delete records" when appropriate.

See feature request #1283
@mgrojo
Copy link
Member

@mgrojo mgrojo commented Jan 4, 2018

I've just implemented the "Delete records" in the context menu for the row number (the vertical header).

I have doubts about deleting columns from the "Browse data" tab. In my opinion, all the database structure operations should be on the "Database Structure" tab.

@justinclift
Copy link
Member

@justinclift justinclift commented Jan 4, 2018

Awesome stuff @mgrojo!

@WiliTest @quole Would you have time to try the nightly build tomorrow, and make sure this works well from your points of view? 😄

@chrisjlocke
Copy link
Contributor

@chrisjlocke chrisjlocke commented Jan 5, 2018

Just a note that the 'delete record' works on the currently selected record, and NOT the record you right click on. This is different behaviour to the 'Duplicate record' in the same menu.

https://screencast-o-matic.com/watch/cFVhluofSQ

So here, I select the record containing 'Amazon' (row 2) but right click row 7 and select 'Duplicate record'. Row 7 is duplicated.

However, if I select a cell in row 7, right click row 9 and select 'Delete record', then row 7 disappears. Makes sense, as that would be what would happen if the 'Delete record' button is clicked, but its not 'right' from a clicky mousey point of view.

Also (linked to this), if there is no selection, right clicking a row and selecting 'delete record' throws up an error that a record must be selected first. Again, the 'duplicate record' option doesn't exhibit this behaviour.

@WiliTest
Copy link
Author

@WiliTest WiliTest commented Jan 5, 2018

That's perfect for the "delete record"!

I understand that deleting the column in the Browse Data tab isn't a good idea. But that would be great to have the possibility to delete/edit a column by right clicking it in the Database Structure tab.

@justinclift
Copy link
Member

@justinclift justinclift commented Jan 5, 2018

@chrisjlocke Good catch. That inconsistency sounds like it'll catch people out.

@mgrojo How easy would it be to adjust, so the record that's right clicked on is the one which gets deleted?

@mgrojo
Copy link
Member

@mgrojo mgrojo commented Jan 5, 2018

It is easy if the right click on a row selects that row. I think it makes sense and I've checked that LibreOffice does so.

@justinclift
Copy link
Member

@justinclift justinclift commented Jan 5, 2018

Cool. That's probably the right way to go then, as it'll keep consistency with the duplicate record behaviour like @chrisjlocke points out. 😄

mgrojo added a commit that referenced this issue Jan 5, 2018
This makes sense from the user point of view, since it remarks that the
options from the menu applies to that row. This change will also fix the
different behaviour of the "Delete record" and "Duplicate record" in the
context menu. See issue #1283
@mgrojo
Copy link
Member

@mgrojo mgrojo commented Jan 5, 2018

Done.

@chrisjlocke
Copy link
Contributor

@chrisjlocke chrisjlocke commented Jan 5, 2018

It is easy if the right click on a row selects that row.

Currently right clicking does NOT highlight the row, or move the selection.
The 'duplicate record' either works on a selection if the mouse is right clicked on a selection, otherwise it uses the row under the mouse.

@mgrojo
Copy link
Member

@mgrojo mgrojo commented Jan 5, 2018

@chrisjlocke I know, but it makes sense to highlight the row to which the operations in the context menu will be applied. This is what I've just implemented. Don't you find it better now?

@chrisjlocke
Copy link
Contributor

@chrisjlocke chrisjlocke commented Jan 5, 2018

Thanks for making the change. I'll test the nightlies when they're built tomorrow.

mgrojo added a commit that referenced this issue Jan 5, 2018
Before bbac655 it was possible to delete
a set of selected rows. This makes it possible again by only selecting the
row if it is not already inside the selected rows. See issue #1283.

Additionally and for coherence, the "Duplicate record" from the context
menu is also made to apply to the list of selected rows. See issue #1090
@mgrojo
Copy link
Member

@mgrojo mgrojo commented Jan 5, 2018

@chrisjlocke Now you'll have to test also #1090 😄

@chrisjlocke
Copy link
Contributor

@chrisjlocke chrisjlocke commented Jan 6, 2018

Thanks @mgrojo - the changes work well.

@justinclift
Copy link
Member

@justinclift justinclift commented Jan 7, 2018

Been thinking over this... I kind of think that being able to right click on a column header then choose "delete column" would be dangerous. As in potential data loss.

I can easily image new-ish users trying it out, thinking that perhaps it just removes the column from the Browse Data display instead of actually nuking it from the table. On the other hand, we do have the "Revert Changes" thing and require people to commit things before they're saved.

Still... I'm kind of inclined to think we should play it safe. Thoughts? 😄

@mgrojo
Copy link
Member

@mgrojo mgrojo commented Jan 7, 2018

@justinclift I'd go for a context menu with a "Delete field" option when right clicking on the field in the "Database structure" tab. There should be a warning like the one we raise when deleting the field inside the Edit Database dialog.

I don't know that part of the code yet, and haven't found the way at the first sight, but I suppose it shouldn't be too difficult.

@chrisjlocke
Copy link
Contributor

@chrisjlocke chrisjlocke commented Jan 7, 2018

I'm kind of inclined to think we should play it safe. Thoughts?

From my point of view, I tend to think that the 'Browse Data' should be just about viewing data, and not manipulating the schema of the database. Deleting rows is one thing (deleting data) but deleting columns (fields) is twinkling with the data, which should be separate In my opinion, of course..

@justinclift
Copy link
Member

@justinclift justinclift commented Jan 7, 2018

I'd go for a context menu with a "Delete field" option when right clicking on the field in the "Database structure" tab.

If you think it's worth it, then sure. We already have a right click context menu on the main table entries (eg "Browse Data", "Modify Table", etc), so it'd probably be similar to that.

Thinking about it... it probably wouldn't hurt, and might be useful. Not sure where the existing code is for deleting fields. If it's all nicely self contained already and it's just a matter of calling it with the right options, that'd be excellent. If it's buried inside the modify table code though... not sure. I can imagine that handling errors (eg violation of foreign key constraints) could be tricky. But, I have no idea. 😄

For the request in this issue though - right click column delete in Browse Data tab - it sounds like we're all in agreement that we shouldn't implement that. 😄

@WiliTest So... killing rows is probably going to be where it's at. Killing columns, nope. 😉

@MKleusberg
Copy link
Member

@MKleusberg MKleusberg commented Aug 16, 2018

I completely agree with @chrisjlocke's argument here and with @justinclift's conclusion. Let's not edit the database schema from a tab which is called "Browsed Data". In a spreadsheet application it makes sense to have both because columns and rows are essentially the same - you could in theory even rotate the table by 90 degrees. In a database I feel like they are too different to be handled in the same way.

As for deleting records by right clicking, that has been implemented by @mgrojo as while ago (right click on the row number, then click "Delete record" in the context menu). Did you had a chance to test that already, @WiliTest? 😄

And should we close this issue then?

@MKleusberg
Copy link
Member

@MKleusberg MKleusberg commented Sep 28, 2018

Closing this issue because we haven't heard about any problems with the current solution. If you feel like I have closed this too early, you can always reopen this issue, @WiliTest 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants