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

Jump to bottom

Fix delete confirmations in workspace dock #18979

Merged
merged 4 commits into from Feb 24, 2017
Merged

Fix delete confirmations in workspace dock #18979

merged 4 commits into from Feb 24, 2017

Conversation

samueljackson92
Copy link
Contributor

Fixes workspace deletion confirmations that are currently broken

To test:

Follow the reproduce steps in the bug report and check it is now fixed

Fixes #18978 .

Reviewer

Please comment on the following (full description):

Code Review
  • Is the code of an acceptable quality?
  • Does the code conform to the coding standards? Is it well structured with small focussed classes/methods/functions?
  • Are there unit/system tests in place? Are the unit tests small and test the a class in isolation?
  • If there are changes in the release notes then do they describe the changes appropriately?
Functional Tests

  • Do changes function as described? Add comments below that describe the tests performed?

  • How do the changes handle unexpected situations, e.g. bad input?

  • Has the relevant documentation been added/updated?

  • Is user-facing documentation written in a user-friendly manner?

  • Has developer documentation been updated if required?

  • Does everything look good? Comment with the ship it emoji but don't merge. A member of @mantidproject/gatekeepers will take care of it.

@samueljackson92 samueljackson92 added Component: GUI Patch Candidate Urgent issues that must be included in a patch following a release labels Feb 23, 2017
@samueljackson92 samueljackson92 added this to the Release 3.10 milestone Feb 23, 2017
@martyngigg martyngigg self-assigned this Feb 24, 2017
martyngigg
martyngigg approved these changes Feb 24, 2017
View reviewed changes
Copy link
Member

@martyngigg martyngigg left a comment •
edited

Choose a reason for hiding this comment

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

Given the current structure of MantidPlot the code changes make sense to me and will at least allow us to reflect other config changes in external code.

I've tested the changes with creating several workspaces both grouped & ungrouped and deleting them with the option both on and off and the application responded as expected.

The state of the option is also preserved correctly between sessions.

LGTM :shipit:

@martyngigg martyngigg added the High Priority An issue or pull request that if not addressed is severe enough to postponse a release. label Feb 24, 2017
@martyngigg
Copy link
Member

I am including this in the patch so that it can help alleviate the problems with accidentally deleting workspaces when the dock is not focussed. See #18976

martyngigg added a commit that referenced this pull request Feb 24, 2017
@martyngigg
Cherry-pick pull request #18979
ab9feec 
Refs #18978 Update dock when config changes

(cherry picked from commit b54bdfb)

Refs #18978 Set confirmations on startup

(cherry picked from commit eca83cf)

Refs #18978 Emit modified signal when settings are read.

(cherry picked from commit f694b70)

Add patch release note
@peterfpeterson peterfpeterson merged commit 0cc7201 into master Feb 24, 2017
@peterfpeterson peterfpeterson deleted the 18978_fix_delete_confirmations branch February 24, 2017 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martyngigg martyngigg martyngigg approved these changes

Assignees

@martyngigg martyngigg

Labels
High Priority An issue or pull request that if not addressed is severe enough to postponse a release. Patch Candidate Urgent issues that must be included in a patch following a release
Projects
None yet
Milestone
Release 3.9.1
Development

Successfully merging this pull request may close these issues.

Fix delete confirmations
3 participants
@samueljackson92 @martyngigg @peterfpeterson