Ticket #9371 (closed: fixed)
New MVP Reflectometry GUI - Base setup
Reported by: | Keith Brown | Owned by: | Harry Jeffery |
---|---|---|---|
Priority: | major | Milestone: | Release 3.3 |
Component: | Reflectometry | Keywords: | |
Cc: | Blocked By: | #9470, #9500, #9501 | |
Blocking: | #9368, #9865, #10222 | Tester: | Owen Arnold |
Description (last modified by Owen Arnold) (diff)
This ticket will have the presenter, a presenter test class, and the abstract view class, as well as any other classes required to make this work to a basic level.
The presenter constructor should take a ITableworkspace and the view and call showTable() from the viewer passing the model as a parameter.
The end result should be able to:
- Load and display a table workspace as a model.
- Create a new table
- Allow to process rows (and sets of rows) in the table via ReflectometryReductionOneAuto
- Allow the user to save
Change History
comment:2 Changed 6 years ago by Nick Draper
- Owner set to Keith Brown
- Status changed from new to assigned
comment:3 Changed 6 years ago by Keith Brown
- Status changed from assigned to inprogress
Refs #9371 View and Presenter basics and Test
The Presenter has been created which assignes a model to a view
The view base calss has been written. This is abstract as the actual view we'll be using will be Qt based
A test for the presenter has been written
Changeset: 6aa7936690caed680f0317d10794c8f16ad52eb8
comment:4 Changed 6 years ago by Keith Brown
Refs #9371 Presenter now subclassed from interface
The presenter is now a child of the new IReflPresenter, as is the new ReflNullMainViewPresenter
Changeset: 893dc433343eac691c508f7e0a6ece7e0cb3ede5
comment:5 Changed 6 years ago by Keith Brown
- Description modified (diff)
There has been a slight change to the ticket's end goal. The gui has to be in a form that can load and display a table workspace as a model before this is deemed complete
The next step is as follows
I mentioned to Owen that the way we've got it now the interface would require a table workspace before it loads, when this interface should be able to create a blank and go from there.
As a result the presenter is being sub-classed from an interface and a Null object made for when there is no model. The constructor is also being overloaded to take a string rather than an already loaded table workspace in which case it checks the ADS and loads it if not in the ADS.
comment:6 Changed 6 years ago by Keith Brown
Refs #9371 Restructuring test slightly
A test needed moving inot a new class
Discovered i was missing system.h from includes as i needed it for the DLLExport
Changeset: 51c39a7f4a35e19ef06fca1bad2277e1df40c96e
comment:7 Changed 6 years ago by Keith Brown
Refs #9371 Updated Class Docs
The doxygen block in the header has been updated.
Changeset: 657e382e9e27deccd4df7cb9fb0f5c9761bd3fb3
comment:9 Changed 6 years ago by Keith Brown
Refs #9371 View and model are now presenter members
the presenter now keeps the view and model as member variables. Load() is used to put the model into the view.
Changeset: c307f078f6ef15d45d4beeaeef0ccdc0919d14e4
comment:10 Changed 6 years ago by Keith Brown
Refs #9371 Interface now loads tables into itself
The interface will now load and display any tableworkspaces in reflectometry format already in the ADS.
Changeset: a02651c25c15e926ef4b63729ee443625d54c55d
comment:11 Changed 6 years ago by Keith Brown
Refs #9371 Unit tests updated
The unit tests have been updated wiht the following:
The null object now tests notify() rather than load() as notify is all the view should do and load() is now private.
The concrete class now tests three fail cases where the table isn't in a valid format.
Changeset: 31e2c2fb8dd14bbdcfefa9cc4b54e6790f9cb3fd
comment:12 Changed 6 years ago by Keith Brown
- Blocking 9373 removed
comment:13 Changed 6 years ago by Owen Arnold
Things to fix:
- Having a creational method like this ITableWorkspace_sptr createbadWorkspace(int typeflag = 1) that returns the product is good. But the naming isn't good. Use full camel case. Using lowercase with underscores is also acceptable.
- The typeflag argument is completely opaque without looking at the body of the function. This is bad. Either provide good descriptive arguments for this creational method, or better yet, have separate functions for each variant that you need. For example create_bad_tableworkspace_too_many_columns(), create_bad_tableworkspace_too_few_columns() This will make the tests much more readable
comment:14 Changed 6 years ago by Keith Brown
Refs #9371 Split usage of the presenter into a loaded and blank.
ReflMainViewPresenter has been made abstract and sublassed to ReflBlankMainViewPresenter (which creates a blank workspace with one row on the fly) and ReflLoadedMainViewPresenter (which does exactly what ReflMainViewPresenter used to do)
The test for ReflMainViewPresenter, has been moved to ReflLoadedMainViewPresenter and has had the fixes owen mentioned applied.
Rejigged the layout on the UI, in leiu of a menubar, there's a temperary row of buttons that will act the same until i can move the functionaity elsewhere.
Changeset: 5a339702fd824fa05e2982693c347c3493393a86
comment:15 Changed 6 years ago by Keith Brown
Refs #9371 Editing and saving tables
Users can now edit and save changes to tables.
Changeset: 857709601cf15b8062970a92d999bec8b4c7e6e6
comment:16 Changed 6 years ago by Keith Brown
Refs #9371 Removed unused test
The test for the base ReflMainViewPresenter class was made redundant when it was subclassed.
Changeset: 17ff7e3f88d1efebab69cad90960947c4cafbea4
comment:17 Changed 6 years ago by Keith Brown
Refs #9371 Added buttons, flags and getters
Added "Add Row" and "Delete Row" buttons to interface
Added row headers to view as the code that would implement them was missing from the model
"Add Row", "Delete Row" and process have had appropriate flags, getters and connections made in the view
Changeset: 63b1bd911b0f451e1547a14ebcc319837c7a6cb0
comment:18 Changed 6 years ago by Keith Brown
Refs #9371 Add row functionality
Add Row will now append a row to the model. The new row will always be at the end for now.
Changeset: 1de63d7deac0d5f5ee367a316f7875cdd2ac7a65
comment:20 Changed 6 years ago by Keith Brown
Refs #9371 Add row, delete row and process
The add row functionality has been fleshed out to allow adding in speciifc places
The delete row functionality has been added and removes only when an entire row is selected
Processing is now possible, however it does contain a few hard-coded hacks to set values at present while owen fixes a fault in CreateTransmissionWorkspaceAuto
Methods to show differeing levels of message box have been added inluding Yes/No question, Critical, Warning and Information
The view can now return a vector<size_t> of selected columns.
Doxygen info has also been added
Changeset: a231f52e020fb1b0246c3fceb5c60afaa641af0d
comment:21 Changed 6 years ago by Keith Brown
Refs #9317 Add and delete row tests
Add row and delete row now have unit tests
Cleaned up a load of gmock warnings by creating a new class specifically for the constructor.
Changeset: 63193658fefb87a9386dd212bc1a7adf022b3ccf
Was marked against wrong ticket, copied here where it should be.
comment:22 Changed 6 years ago by Keith Brown
Refs #9371 Working on processing tests
Processing testing had been started, although this ticekt has now become blocked by the problem that the two Auto algorithms i use can't be accessed from a unit test, so that'll need fixed before i can continue.
Changeset: bb40c89d212ff717b54f260673c3b120fbc44a09
comment:25 Changed 6 years ago by Keith Brown
- Owner changed from Keith Brown to Owen Arnold
- Milestone changed from Release 3.2 to Backlog
not going to get this done before i leave, given to Owen
comment:28 Changed 6 years ago by Harry Jeffery
Refs #9371 Fix compilation
Changeset: 64812eb3c2625e38334918ae138dc555373be7ab
comment:29 Changed 6 years ago by Harry Jeffery
Refs #9371 Fix initialiser order warning
Changeset: c9f6c2dc5e2114ce4c0a2c6d3c487d50edce468a
comment:30 Changed 6 years ago by Harry Jeffery
Refs #9371 Fix private slots indentation
Changeset: e1531c228c3f3c0d399e3f6077e305c893985406
comment:31 Changed 6 years ago by Harry Jeffery
Refs #9371 Re-layout Reflectometry UI
The existing design was historical. This commit dispenses with some obsolete items and reorganises the existing items. The new layout is still a little bit rough, so there's room for improvement. But it's a step in the right direction.
Changeset: e75ded6f546a0b3fd35877ce6c4d947e91f4918b
comment:32 Changed 6 years ago by Harry Jeffery
Refs #9371 Fix a few typos
Changeset: c8b90c5e1c5bb3c7f8084d0479a0d4c5d6bbd856
comment:33 Changed 6 years ago by Harry Jeffery
Refs #9371 Ensure no refl presenter notifications are ignored
Changeset: 0eb4b2145b4690f8e7eb706e1a19af4785625c9c
comment:34 Changed 6 years ago by Harry Jeffery
Refs #9371 Remove some magic numbers
Changeset: 61ffc2c8989a8caa26f7faa58320e74c0815600a
comment:35 Changed 6 years ago by Harry Jeffery
Refs #9371 Resolve a gcc warning
Changeset: abf8611ad74386705506c4d9f85e29657ab36eb3
comment:36 Changed 6 years ago by Harry Jeffery
Refs #9371 Clean up superfluous logic in deleteRow()
Changeset: 319f93e6bd3d14519355d7d55121a64b78cfe584
comment:37 Changed 6 years ago by Harry Jeffery
Refs #9371 Refactor makeTransWS
Changeset: 73929ffc63850ab60cebf74940617485de6e4896
comment:38 Changed 6 years ago by Harry Jeffery
Refs #9371 Fix caching bug.
When there is only one row in the reflectometry gui, updating a value on it does not cause the table view to be updated. This was a caching related problem. This commit fixes that problem and optimises the dataChanged signal, so that only items that have been updated need to be fetched by the table view.
Changeset: 6e1e5ea96a32ddd3906f5cafeedcc008e3df1cb2
comment:39 Changed 6 years ago by Harry Jeffery
Refs #9371 Tidy up the Refl Gui a little more.
Changeset: fc7edfd56ee89bb382fb1e7fc3ae2f248eed26a7
comment:40 Changed 6 years ago by Harry Jeffery
Refs #9371 Refactor out some more magic numbers.
Changeset: 959648089395a51446a7e868fbd821a5f66bcbed
comment:41 Changed 6 years ago by Harry Jeffery
Refs #9371 Strip whitespace in Refl Gui table.
Changeset: ebd419ef1aecc92da1954db33563c0f774cd3dee
comment:43 Changed 6 years ago by Harry Jeffery
Refs #9371 Fix error when processing rows
Changeset: 5fff6522e73200713927e4b12b3094df683c7f2b
comment:44 Changed 6 years ago by Harry Jeffery
Refs #9371 Fix warning in unit tests
Changeset: 74ef095808621d48c2e6c80c5f998ebdbc15354e
comment:45 Changed 6 years ago by Harry Jeffery
Refs #9371 Refactor ReflMainView flag interface
Before, each flag had its own boolean, and its own method to fetch that boolean. This was all very well, but then to add a new flag you'd have to add a new method in every child view, and then handle that method in every presenter that used the view. You'd also then have to fix every single unit test for the presenters. It was a bit brittle.
This commit replaces that with a single method getFlag which pops the first flag from an internal queue, a flag now being an enum. The presenter can check if there are any flags waiting to be processed by calling flagSet.
Presenters just fetch the flags until there are no more, and handle them as appropriate. Views just add the right flag to their internal queue when they like.
Changeset: c4ed068e6e0cbc99bd4d74df3f04cb004a15ac38
comment:46 Changed 6 years ago by Harry Jeffery
Refs #9371 Tidy ReflMainViewPresenter processing
Changeset: 53ada0700ac91d9060d9f64b93a6288373b6d6ad
comment:47 Changed 6 years ago by Harry Jeffery
Refs #9371 Make ReflMainView::askUserString more flexible
Changeset: e168b80a50a16cb02af10506ca64ce524ab66b53
comment:48 Changed 6 years ago by Harry Jeffery
Merge branch 'master' into feature/9371_Refl_MVP_Base_Setup
Refs #9371.
Conflicts:
Code/Mantid/MantidQt/CustomInterfaces/CMakeLists.txt
Changeset: 0b2e983f2a88c575ff48dd19b4ffd880d0ebbea6
comment:49 Changed 6 years ago by Harry Jeffery
Refs #9371 Fix broken unit tests
I forgot to update the tests after changing the type signature of askUserString.
Changeset: 69e158149d19eb38ecc6ea4459d9bd43505cde9a
comment:50 Changed 6 years ago by Harry Jeffery
Refs #9371 Fix Doxygen documentation warnings
Changeset: 529e453ac4c314eea86883dee8a7e42070caeb6b
comment:51 Changed 6 years ago by Harry Jeffery
Refs #9371 Fix doxygen documentation warnings 2
Changeset: 50075f0e8db77d50eeb3c2e8d2ba032685a7fdde
comment:52 Changed 6 years ago by Harry Jeffery
- Status changed from inprogress to verify
- Resolution set to fixed
comment:53 Changed 6 years ago by Owen Arnold
- Status changed from verify to verifying
- Tester set to Owen Arnold
comment:55 Changed 6 years ago by Harry Jeffery
- Status changed from verifying to closed
Merge branch 'master' into feature/9371_Refl_MVP_Base_Setup
Refs #9371.
Conflicts:
Code/Mantid/MantidQt/CustomInterfaces/CMakeLists.txt
Full changeset: 0b2e983f2a88c575ff48dd19b4ffd880d0ebbea6
comment:56 Changed 6 years ago by Owen Arnold
Merge remote-tracking branch 'origin/feature/9371_Refl_MVP_Base_Setup'
Full changeset: 2af01ab02ce817b3db414efb67d33a4d30fc7838
comment:57 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 10214