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

Initial implementation of the undelete mechanism for fitting tabs - PR #2778 rebased to main #3140

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

rozyczko
Copy link
Member

Description

Cherry picked changes to merge with main without much drama
Discussion + review + comments in: #2778

User request addressed - it is easy (too easy) to delete a fitting tab. It would be good to be able to recover it somehow.

This PR is a cruft before the proposed Undo stack is implemented, allowing for more streamlined action of recovering UI errors.

Here, we just store the deleted tab and if needed, populate the perspective with it.
The key shortcut to restore the deleted tab is Ctrl-T, akin to the web browser standard.

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.

Review Checklist:

[if using the editor, use [x] in place of [ ] to check a box]

Documentation (check at least one)

  • There is nothing that needs documenting
  • Documentation changes are in this PR
  • There is an issue open for the documentation (link?)

Installers

  • There is a chance this will affect the installers, if so
    • Windows installer (GH artifact) has been tested (installed and worked)
    • MacOSX installer (GH artifact) has been tested (installed and worked)

Licencing (untick if necessary)

  • The introduced changes comply with SasView license (BSD 3-Clause)

@krzywon

This comment was marked as resolved.

Copy link
Contributor

@krzywon krzywon left a comment

Choose a reason for hiding this comment

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

On 2nd review of this same branch, many of the issues I noted in my comment are difficult to reproduce. I still can't approve until the data deletion issue is fixed, as noted below, but in the meantime, I'm going to try to recreate the issues I saw earlier.

src/sas/qtgui/Perspectives/Fitting/FittingPerspective.py Outdated Show resolved Hide resolved
@@ -349,8 +397,15 @@ def tabCloses(self, index):
"""
Update local bookkeeping on tab close
"""
# update the last-tab closed information
# this should be done only for regular fitting
if not isinstance(self.tabs[index], ConstraintWidget) and \
Copy link
Contributor

Choose a reason for hiding this comment

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

Deleting data, then restoring a tab restores the tab with the deleted data set. This should be handled in dataDeleted below.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really. Deleting data happens from quite a few locations and shouldn't be polluted with the tab recovery code, which is not really deleting the data...

@rozyczko
Copy link
Member Author

  • Close the last tab and the fitting perspective goes blank. The '+' button is no longer available, but CTRL-T works to restore the last closed tab.

This is explicitly not supposed to happen. The delete tab handler checks for it. If you are able to reproduce this behaviour, please make an issue.

@krzywon
Copy link
Contributor

krzywon commented Oct 29, 2024

  • Close the last tab and the fitting perspective goes blank. The '+' button is no longer available, but CTRL-T works to restore the last closed tab.

This is explicitly not supposed to happen. The delete tab handler checks for it. If you are able to reproduce this behaviour, please make an issue.

Trying again, I cannot reproduce the issues I saw. I've hidden and resolved that comment.

At this point, the only remaining issue I see is, when data is deleted, the tab(s) with that data are closed, as expected, but the each tab closed during the deletion process is stored in self.lastTabClosed. This allows a tab with removed data to be reopened.

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