Skip to content

Fix delete confirmations in workspace dock#18979

Merged
peterfpeterson merged 4 commits into
masterfrom
18978_fix_delete_confirmations
Feb 24, 2017
Merged

Fix delete confirmations in workspace dock#18979
peterfpeterson merged 4 commits into
masterfrom
18978_fix_delete_confirmations

Conversation

@samueljackson92
Copy link
Copy Markdown
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
Copy link
Copy Markdown
Member

@martyngigg martyngigg left a comment

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
Copy Markdown
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
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

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

Development

Successfully merging this pull request may close these issues.

Fix delete confirmations

3 participants