Ticket #10237 (closed: fixed)
Put menu options on new ReflGui
Reported by: | Owen Arnold | Owned by: | Harry Jeffery |
---|---|---|---|
Priority: | critical | Milestone: | Release 3.3 |
Component: | Reflectometry | Keywords: | |
Cc: | Blocked By: | #10223, #10236, #10295, #10302, #10376 | |
Blocking: | #10238, #10347 | Tester: | Federico M Pouzols |
Description
Solving the previous issues (see blockers) will mean we can put menus onto the gui.
Remember that in passive MVP, we do not want the views to know anything about processing logic. They must not therefore perform conditional branching based on menu selections, and they should not even launch menus themselves. One way around this would be to create a new MVP group for say the ReflGuiOptions dialog. This could be kicked off by the main presenter.
Change History
comment:5 Changed 6 years ago by Harry Jeffery
- Status changed from assigned to inprogress
- Blocked By 10302 added
comment:9 Changed 6 years ago by Harry Jeffery
Refs #10237 Add empty Options dialog
The user is now able to open an empty options dialog. The dialog uses the QtReflMainView as its parent and gets a copy of the shared pointer to the presenter.
The ui of the new dialog is defined by ReflOptionsDialog.ui.
Changeset: 2f1b158caa2767501002e7c618ef583579ca28e4
comment:10 Changed 6 years ago by Harry Jeffery
Refs #10237 Add options to ReflMainViewPresenter
Changeset: 9e3ecceacd6a9736af65a846d580edbfc4c99126
comment:11 Changed 6 years ago by Harry Jeffery
Refs #10237 Use options in Refl UI
To demonstrate the basic functionality a "WarnProcessAll" options has been added. This allows the user to decide whether or not they wish to be warned when they try to process all the rows (by not selecting any before hitting process).
Changeset: 3cd1e03308093d4868eef31f86fbf7d63d259a4a
comment:12 Changed 6 years ago by Harry Jeffery
Refs #10237 Expedite options dialog deallocation
Changeset: 3af0c90d1dc0fca4df5bbb1e43ecebda70823de8
comment:13 Changed 6 years ago by Harry Jeffery
Refs #10237 Store options as QVariants
This way we don't need to handle type conversion ourselves. We can let QVariant take care of that for us.
Changeset: f75a370139c3defebd42ef01c4290cff66b0e26a
comment:14 Changed 6 years ago by Harry Jeffery
Refs #10237 Persist options using QSettings
Options are saved in the group: Mantid/CustomInterfaces/ISISReflectometry
Changeset: 57e83ef61e6be48bef9725b0a5b76ab4929e57aa
comment:15 Changed 6 years ago by Harry Jeffery
- Status changed from inprogress to verify
- Resolution set to fixed
Testing
This ticket provides a dialog for configuring options/settings for the ISIS Reflectometry UI. A simple option to enable/disable the "Are you sure you want to process all rows?" prompt has been added to it for the purposes of testing.
- Verify that the dialog behaves intuitively
- Verify that the settings persist between runs of the application
- Verify that the "Warn when processing all rows" setting works as expected
- Review changes
- Verify no unit tests have been broken
Branch: https://github.com/mantidproject/mantid/compare/feature/10237_refl_ui_options_menu
comment:16 Changed 6 years ago by Federico M Pouzols
- Status changed from verify to verifying
- Tester set to Federico M Pouzols
comment:17 Changed 6 years ago by Federico M Pouzols
- Status changed from verifying to closed
This looks all fine. The unit tests pass, and the simple options dialog behaves well. The new code looks well structured and it comes with good documentation. More specifically about the dialog:
- the "warn" option persists between different runs
- the warning is shown when it should. A detail: do you also want to show this warning when there's very few or just one row?
- the dialog itself behaves as I would expect, even though the "new refl" is still growing I'd guess. I was missing for example a quit/exit entry.
comment:18 Changed 6 years ago by Federico Montesino Pouzols
Merge remote-tracking branch 'origin/feature/10237_refl_ui_options_menu'
Full changeset: 644714f30ffb0612ddd8dca6737907316d4a6cf6
comment:19 Changed 6 years ago by Harry Jeffery
Refs #10237 Include QVariant rather than declare
Changeset: b7309429cef08e03963cab525ff0a3f240fb7250
comment:20 Changed 6 years ago by Martyn Gigg
Merge branch 'feature/10237_refl_ui_options_menu'
Full changeset: 71f4ed4bcbd6059301f1dc7f6d040b2696e2a4a7
comment:21 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 11079