Ticket #10431 (closed: fixed)
ReflMainViewPresenter goes behind the model's back
Reported by: | Harry Jeffery | Owned by: | Harry Jeffery |
---|---|---|---|
Priority: | major | Milestone: | Release 3.3 |
Component: | Reflectometry | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | #10440 | Tester: | Owen Arnold |
Description
Currently the presenter does not use the ReflTableModel to manipulate the table, it goes through the table workspace itself. This stops the model from receiving proper updates on what has changed, and therefore from updating the view correctly. To get around this currently, every time the presenter changes the table it is reloaded into the view. This means that selections and table settings cannot be preserved.
Instead of passing the ReflMainView a table workspace pointer, ReflMainViewPresenter should pass it a ReflTableModel which it shall use itself, allowing the table to be correctly updated.
Change History
comment:2 Changed 6 years ago by Harry Jeffery
Refs #10431 Create default table in helper
Changeset: bef0927e6d57f751dc4d820375cdeb9034f771da
comment:3 Changed 6 years ago by Harry Jeffery
Refs #10431 Replace size_t with int
Qt uses int instead of size_t for row indices. Change our usage to match Qt's to prepare for using QReflTableModel.
Changeset: 66971df5187aff6afdc3b3d652c27c106e4e58c5
comment:4 Changed 6 years ago by Harry Jeffery
Refs #10431 Pass QReflTableModel to QtReflMainView
The view will now automatically stay in sync with the model, because we're not going behind the model's back in the presenter.
Changeset: 14d9842b42c886df1d1f2d711e5bde93017bae79
comment:7 Changed 6 years ago by Harry Jeffery
Refs #10431 Resynchronise documentation
Changeset: 94444493c680411e8259072103c291c549a64440
comment:8 Changed 6 years ago by Harry Jeffery
Refs #10431 Update unit tests
Changeset: 4e5752a0c0d85e6a42e7c3ccad5dd0ecec113683
comment:9 Changed 6 years ago by Harry Jeffery
Merge branch 'master' into feature/10431_refl_presenter_use_model
Refs #10431
Conflicts:
Code/Mantid/MantidQt/CustomInterfaces/test/ReflMainViewPresenterTest.h
Changeset: 1434c96f42676b13ee7f44d50671fa5fc7e088b7
comment:10 Changed 6 years ago by Harry Jeffery
- Status changed from inprogress to verify
- Resolution set to fixed
Testing
The changes introduced by this ticket are subtle, but big wins for usability. When performing actions on the model the users' selection will now be preserved and moved as appropriate. This is very useful when inserting rows in-place, or grouping and then processing.
- Verify unit tests are passing
- Inspect changes
- Play with row manipulation (especially with the context menu)
- Your selection should be preserved for all actions (deleting will of course clear your selection)
Branch: https://github.com/mantidproject/mantid/compare/feature/10431_refl_presenter_use_model
comment:12 Changed 6 years ago by Owen Arnold
- Status changed from verify to verifying
- Tester set to Owen Arnold
comment:13 Changed 6 years ago by Owen Arnold
I agree with these changes, but do not agree with specifying high levels of precision as this is going to be brittle. https://github.com/mantidproject/mantid/blob/1434c96f42676b13ee7f44d50671fa5fc7e088b7/Code/Mantid/MantidQt/CustomInterfaces/test/ReflMainViewPresenterTest.h#L713 use http://cxxtest.com/guide.html#ts_assert_delta instead where this kind of thing has been put in place.
comment:14 Changed 6 years ago by Owen Arnold
- Status changed from verifying to reopened
- Resolution fixed deleted
comment:15 Changed 6 years ago by Harry Jeffery
- Status changed from reopened to inprogress
Refs #10431 Allow some variance in expected DQQ
Changeset: bcd6011051db70aa1a2838b9988a011f91b614af
comment:16 Changed 6 years ago by Harry Jeffery
- Status changed from inprogress to verify
- Resolution set to fixed
comment:17 Changed 6 years ago by Harry Jeffery
Merge branch 'master' into feature/10431_refl_presenter_use_model
Refs #10431
Conflicts:
Code/Mantid/MantidQt/CustomInterfaces/inc/MantidQtCustomInterfaces/ReflMainView.h Code/Mantid/MantidQt/CustomInterfaces/src/ReflMainViewPresenter.cpp Code/Mantid/MantidQt/CustomInterfaces/test/ReflMainViewMockObjects.h
Changeset: ebd4ecb968330cbcb34c539c7e71b8a0d5bf5506
comment:20 Changed 6 years ago by Owen Arnold
- Status changed from verify to reopened
- Resolution fixed deleted
Needs merge conflicts resolved.
comment:21 Changed 6 years ago by Harry Jeffery
- Status changed from reopened to inprogress
Merge branch 'master' into feature/10431_refl_presenter_use_model
Refs #10431
Conflicts:
Code/Mantid/MantidQt/CustomInterfaces/test/ReflMainViewMockObjects.h
Changeset: 69a06db56ced9f80876bb904731f016371bc5ec3
comment:22 Changed 6 years ago by Harry Jeffery
- Status changed from inprogress to verify
- Resolution set to fixed
comment:23 Changed 6 years ago by Harry Jeffery
- Status changed from verify to closed
Merge branch 'master' into feature/10431_refl_presenter_use_model
Refs #10431
Conflicts:
Code/Mantid/MantidQt/CustomInterfaces/test/ReflMainViewPresenterTest.h
Full changeset: 1434c96f42676b13ee7f44d50671fa5fc7e088b7
comment:24 Changed 6 years ago by Harry Jeffery
Merge branch 'master' into feature/10431_refl_presenter_use_model
Refs #10431
Conflicts:
Code/Mantid/MantidQt/CustomInterfaces/inc/MantidQtCustomInterfaces/ReflMainView.h Code/Mantid/MantidQt/CustomInterfaces/src/ReflMainViewPresenter.cpp Code/Mantid/MantidQt/CustomInterfaces/test/ReflMainViewMockObjects.h
Full changeset: ebd4ecb968330cbcb34c539c7e71b8a0d5bf5506
comment:25 Changed 6 years ago by Harry Jeffery
Merge branch 'master' into feature/10431_refl_presenter_use_model
Refs #10431
Conflicts:
Code/Mantid/MantidQt/CustomInterfaces/test/ReflMainViewMockObjects.h
Full changeset: 69a06db56ced9f80876bb904731f016371bc5ec3
comment:26 Changed 6 years ago by Owen Arnold
Merge remote-tracking branch 'origin/feature/10431_refl_presenter_use_model'
Full changeset: d48fb15186e08856c0a60b4ac6a543c06da1e211
comment:27 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 11273
Refs #10431 Extend QReflTableModel
Changeset: 6f5a68928ca55f2cf5f786324006d325bd60e07a