Ticket #10223 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

New Refl UI improvements

Reported by: Owen Arnold Owned by: Harry Jeffery
Priority: critical Milestone: Release 3.3
Component: Reflectometry Keywords:
Cc: Blocked By: #10222, #10250
Blocking: #9473, #10235, #10237, #10239 Tester: Anders Markvardsen

Description (last modified by Owen Arnold) (diff)

Here are some fixes/additions I think we can make straight off.

Most pressing

  • Table should occupy full window area. Lots of white space around the table for some reason.
  • Enable stitching of results via the group column

Less pressing issues

  • Add a progress bar. Increment the progress for each row processed.
  • A highlight rows right-click group. Or group button would be useful, which would automatically write to the group column.
  • Group column should only accept integers.
  • Process button should be 'normal sized'.
  • 'Add Row' adds a row above the selected one. This is strange. It should be below.
  • See about improving the current style. For example alternate background colours on rows ​http://qt-project.org/doc/qt-4.8/stylesheet-examples.html#customizing-qtableview

Test Considerations

  • Unit tests should be extensive and cover new functionality in the presenters.
  • Unit tests should be fast to execute
  • Unit tests are documentation so should be easy to read. Add documentation liberally and use TSM where the results need explaining.

Change History

comment:1 Changed 6 years ago by Owen Arnold

  • Status changed from new to assigned
  • Owner set to Harry Jeffery
  • Component changed from Framework to Reflectometry
  • Milestone changed from Backlog to Release 3.3

comment:2 Changed 6 years ago by Owen Arnold

  • Priority changed from major to critical
  • Description modified (diff)

comment:3 Changed 6 years ago by Harry Jeffery

  • Status changed from assigned to inprogress

comment:4 Changed 6 years ago by Harry Jeffery

Refs #10223 Lower initial Refl UI size

Changeset: 55cc6182478f33779bbd1968154e53b049de145e

comment:5 Changed 6 years ago by Harry Jeffery

Refs #10223 Decrease whitespace in Refl UI

Changeset: 9c52ab49512b389244e5e8aff79bfe7c1eec84a8

comment:6 Changed 6 years ago by Harry Jeffery

Refs #10223 Normalise Refl UI button sizing

Changeset: 23548c1f476b93ef52073f8f7b6074db082a1d06

comment:7 Changed 6 years ago by Harry Jeffery

Refs #10223 Add basic input validation

Instead of processing failing mid-way through due to invalid input, validate the input first, then start processing.

Changeset: 77698d34c5d0701fa5258d18899e509586183c82

comment:8 Changed 6 years ago by Harry Jeffery

Refs #10223 Improve const-ness

Changeset: 862b3b506545b621109708976b2f21c2b2527796

comment:9 Changed 6 years ago by Harry Jeffery

Refs #10223 Add a progress bar to the Refl UI

Changeset: ee069df75a9f9c46bf9bf231eac34b056304d519

comment:10 Changed 6 years ago by Harry Jeffery

Refs #10223 Add new rows after the current selection

Changeset: 5a16d93cf6e4c6a0416dcd06e92e3d7546d7286b

comment:11 Changed 6 years ago by Owen Arnold

  • Blocking 10235 added

comment:12 Changed 6 years ago by Harry Jeffery

Revert "Refs #10223 Add new rows after the current selection"

Commit 5a16d93c breaks some unit tests and it's too late in the day to repair them now.

This reverts commit 5a16d93cf6e4c6a0416dcd06e92e3d7546d7286b.

Changeset: 0e0e34bf3e5a95fe4739e3fbc3f3c2f440477f61

comment:13 Changed 6 years ago by Harry Jeffery

Refs #10223 Fix warnings in Refl UI test

Changeset: 09729751a813cf18fe92e7dbfc967c6dab8e8528

comment:14 Changed 6 years ago by Owen Arnold

  • Blocking 10237 added

comment:15 Changed 6 years ago by Harry Jeffery

Refs #10223 Fix build problems

  • Fix static analysis problems found in validateRows
  • Temporarily disableReflLoadedMainViewPresenterTest::testProcessAll while it is debugged.

Changeset: ba705e8a7c9b4cc467fbec59644a3f8903819602

comment:16 Changed 6 years ago by Owen Arnold

  • Blocking 10239 added

comment:17 Changed 6 years ago by Harry Jeffery

Refs #10223 Add instrument selection controls to Refl UI

Changeset: 0b01dc6f03e1e0e6376763568544b84a4b911d89

comment:18 Changed 6 years ago by Harry Jeffery

Refs #10223 Load runs with respect to selected instrument

Changeset: b0c516f1dd28cd8d1f8b14e952920ca649ef5273

comment:19 Changed 6 years ago by Harry Jeffery

Refs #10223 Minor Refl UI tweaks

Changeset: 10bfe8aed60d38071bfb0eb38472ec6555685089

comment:20 Changed 6 years ago by Harry Jeffery

Refs #10223 Update unit tests to pass

The unit tests have not yet been thoroughly reviewed. They are due to be heavily rewritten and documented to test the different use cases in a thorough and easy to follow way.

Changeset: bad5edeb711e2a26ca7ed95d4b80852d5ff53ed5

comment:21 Changed 6 years ago by Harry Jeffery

Refs #10223 Resolve some build issues

  • Fix an undocumented parameter flagged by doxygen
  • Don't try to process INTER13469-INTER13470. They're not available to the build services' unit tests.

Changeset: 3ae2e68ff8ac2f2bee4348235f3219f0853a0837

comment:22 Changed 6 years ago by Harry Jeffery

  • Blocked By 10250 added

comment:23 Changed 6 years ago by Harry Jeffery

  • Blocking 9473 added

comment:24 Changed 6 years ago by Harry Jeffery

Refs #10223 Do not run rebinning algorithms automatically

Changeset: 1335c8f3c921b6794a4f95248e26a075548ca039

comment:25 Changed 6 years ago by Harry Jeffery

Refs #10223 Implement workspace stitching in Refl UI

This ticket causes process to stitch processed groups together using Stitch1DMany.

Changeset: bad470fa484969fecbf8a702128f5d72ede6f0cd

comment:26 Changed 6 years ago by Harry Jeffery

Refs #10223 Fix minor error in fetchRun

fetchRun was not fetching loaded runs from the ADS correctly because it was using incorrect workspace names to look for them.

Changeset: e3099035725014ac3208f835890ac1759ee691c1

comment:27 Changed 6 years ago by Harry Jeffery

Refs #10223 Add tooltips to Refl UI

Changeset: e4c7a64d126b56ee932a3faf3ee5ef32e0995041

comment:28 Changed 6 years ago by Harry Jeffery

Refs #10223 Alternate Refl UI table row colors

Changeset: 4147653f61423246c724eb5470daa10fedf6dde3

comment:29 Changed 6 years ago by Harry Jeffery

Refs #10223 Clean up ReflBlankMainViewPresenterTest

Changeset: cff0ebd9babb087e952650608bd70b3721eab604

comment:30 Changed 6 years ago by Harry Jeffery

Refs #10223 Clean up ReflLoadedMainViewPresenterTest

Changeset: 3eb782bb178b46558d6b4de08533f48b95cd8858

comment:31 Changed 6 years ago by Harry Jeffery

Refs #10223 Update Refl UI processing unit test

Changeset: 8eff698fe7307fb437738f2b0a10ca1991e5615f

comment:32 Changed 6 years ago by Harry Jeffery

Refs #10223 Tidy up test workspace creation

Changeset: 738db45474211d9508fc328aecca8114afef32c7

comment:33 Changed 6 years ago by Harry Jeffery

Refs #10223 Resolve some static analysis warnings

Changeset: bbfa4e78adda5ab8b99b93e71e996a4c7010ff77

comment:34 Changed 6 years ago by Harry Jeffery

Refs #10223 Factor out magic numbers in tests

Changeset: 01abc02d23a950c084f7fa70b771b67fe927ba7a

comment:35 Changed 6 years ago by Harry Jeffery

Refs #10223 Merge mock objects together

  • Merge AddDelProcView into MockView
  • Remove ConstructView

Changeset: ca7597641effafb94344d424703971274f236f96

comment:36 Changed 6 years ago by Harry Jeffery

Refs #10223 Don't place workspaces into ADS unnecessarily

Changeset: fc0681633e729d8ef3044165104474fcf79a18c0

comment:37 Changed 6 years ago by Harry Jeffery

Refs #10223 Attempt to fix Win7 build

Changeset: 3a763c5f273880cc9f673c78201c6968a8297f2b

comment:38 Changed 6 years ago by Harry Jeffery

Refs #10223 Add group rows button

  • Add group rows button to Refl UI
  • Add unit test for group rows button

Changeset: 6440b4c7b6ded22a0b4c9d6b3a56e66519eafb73

comment:39 Changed 6 years ago by Harry Jeffery

Refs #10223 Tidy up BlankReflMainViewPresenterTest

  • Remove superfluous asserts
  • Factor out magic numbers
  • Reorder asserts for readability
  • Correct some whitespace

Changeset: a0858c6ac19a2eb31a81983e0988d45e1b9de5fd

comment:40 Changed 6 years ago by Harry Jeffery

  • Status changed from inprogress to verify
  • Resolution set to fixed

Testing

All of the bullet points in the ticket have been resolved with the exception of having the "add rows" button append rows, instead of prepending them. This is because it requires further discussion with Owen. It is not a pressing issue so please disregard it for this ticket.

  • Verify each bullet point has been resolved
  • Verify builds and tests are passing
  • Inspect code

Branch: ​https://github.com/mantidproject/mantid/compare/feature/10223_new_refl_ui_improvements

For testing it is recommended you create a tbl file with these contents: 

13460,0.7,"13463,13464",0.01,0.06,13462,2.3,"13463,13464",0.035,0.3,,,,,,0.04,
13469,0.7,"13463,13464",0.01,0.06,13470,2.3,"13463,13464",0.035,0.3,,,,,,0.04,

and use LoadReflTBL to load it as a TableWorkspace that can be used by the new Reflectometry UI.

comment:41 Changed 6 years ago by Anders Markvardsen

  • Status changed from verify to verifying
  • Tester set to Anders Markvardsen

comment:42 Changed 6 years ago by Harry Jeffery

  • Status changed from verifying to closed

Merge branch 'master' into feature/10223_new_refl_ui_improvements

Full changeset: ​691708be5188e7bc5bbf27edf866c96e1fd7c079

comment:43 Changed 6 years ago by Anders Markvardsen

Merge remote-tracking branch 'origin/feature/10223_new_refl_ui_improvements'

Full changeset: ​0e7ad927d4b27b3a7590cf8814defc016628d18d

comment:44 Changed 6 years ago by Harry Jeffery

Refs #10223 Add getRunNumber method

Changeset: 8494e1122fc84e04272a7e570867d51db6752910

comment:45 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue ​11065

Note: See TracTickets for help on using tickets.