Simulation wizard: Add substances page

leendertvanwolfswinkel commented 7 months ago

Mockup:

Changes to the design:

Remove the ID column
Add a units column (free text field with limited length, see api endpoint (staging))

Example script:

simulate.zip

hoanphungt commented 5 months ago

Current UI:

Lukasz's comment on the current UI:

Code looks good, but I have doubts about the UX design. Wouldn't it be better to have a wider table and Add new substance button to open small input dialog to enter name and substance units? I think it would look a bit better and would be more intuitive. What's your opinion @leendertvanwolfswinkel ?

leendertvanwolfswinkel commented 4 months ago

I agree with @ldebek comment on the width of the table @hoanphungt I think the original design is the best, as we have just discussed.

[x] Plus button adds a (empty) row to the bottom of the table
[x] Cells are editable directly
[x] Minus button deletes the row(s) that are selected

Would it be possible to color the "Name" cells orange if they are empty, and disable the "Next" button if not all substances are valid (i.e. have a name)?

hoanphungt commented 4 months ago

@leendertvanwolfswinkel I replaced the plus and minus icons by using texts "Add" and "Remove". The default icons from Qt somehow doesn't work for me. Cells are now editable directly.

Rows with empty names will be ignored so it doesn't matter. There are also 2 checks for duplicate names and units length (max 16 characters).

leendertvanwolfswinkel commented 4 months ago

@hoanphungt some points of feedback:

[x] Please make this the empty state (1 row already added to the table):
[x] Please make the "name" column automatically adjust its width to the cell contents, so it doesn't look like this:

But like this:

[x] I suggest we set the unit widget's max length to 16, so that it is not even possible for users to type longer unit names -> max length is already 16

leendertvanwolfswinkel commented 4 months ago

@hoanphungt I tested and got the following error:

See this video:

https://github.com/nens/threedi-api-qgis-client/assets/26224299/fd0fb928-93dd-43e9-b361-5af6c9c3a38d

An error has occurred while executing Python code: 

AttributeError: 'SimulationWizard' object has no attribute 'laterals_page' 
Traceback (most recent call last):
  File "C:\Users\leendert.vanwolfswin\AppData\Roaming\3Di\QGIS3\profiles\default/python/plugins\threedi_models_and_simulations\widgets\simulation_wizard.py", line 264, in handle_item_changed
    self.update_substances()
  File "C:\Users\leendert.vanwolfswin\AppData\Roaming\3Di\QGIS3\profiles\default/python/plugins\threedi_models_and_simulations\widgets\simulation_wizard.py", line 283, in update_substances
    self.parent_page.parent_wizard.laterals_page.main_widget.setup_substance_concentrations()
AttributeError: 'SimulationWizard' object has no attribute 'laterals_page'

Python version: 3.9.18 (heads/master:5eba59e, Feb  1 2024, 20:02:10) [MSC v.1929 64 bit (AMD64)] 
QGIS version: 3.34.4-Prizren Prizren, decd43dc 

Python Path:
C:\Users\leendert.vanwolfswin\AppData\Roaming\3Di\QGIS3\profiles\default\python\plugins\threedi_results_analysis\deps
C:\Users\leendert.vanwolfswin\AppData\Roaming\3Di\QGIS3\profiles\default/python/plugins\threedi_beta_processing
C:/PROGRA~1/3DIMOD~1.34/apps/qgis-ltr/./python
C:\Users\leendert.vanwolfswin\AppData\Roaming\3Di\QGIS3\profiles\default/python
C:\Users\leendert.vanwolfswin\AppData\Roaming\3Di\QGIS3\profiles\default/python/plugins
C:/PROGRA~1/3DIMOD~1.34/apps/qgis-ltr/./python/plugins
C:\PROGRA~1\3DIMOD~1.34\apps\grass\grass83\etc\python
C:\PROGRA~1\3DIMOD~1.34\bin
C:\PROGRA~1\3DIMOD~1.34\bin\python39.zip
C:\PROGRA~1\3DIMOD~1.34\apps\Python39\DLLs
C:\PROGRA~1\3DIMOD~1.34\apps\Python39\lib
C:\Users\leendert.vanwolfswin\AppData\Roaming\Python\Python39\site-packages
C:\PROGRA~1\3DIMOD~1.34\apps\Python39
C:\PROGRA~1\3DIMOD~1.34\apps\Python39\lib\site-packages
C:\PROGRA~1\3DIMOD~1.34\apps\Python39\lib\site-packages\win32
C:\PROGRA~1\3DIMOD~1.34\apps\Python39\lib\site-packages\win32\lib
C:\PROGRA~1\3DIMOD~1.34\apps\Python39\lib\site-packages\Pythonwin
C:\Users\leendert.vanwolfswin\AppData\Roaming\3Di\QGIS3\profiles\default/python
C:\Users\leendert.vanwolfswin\AppData\Roaming\3Di\QGIS3\profiles\default\python\plugins\lizard_qgis_plugin\deps\threedi_scenario_downloader-1.4-py3-none-any.whl
C:\Users\leendert.vanwolfswin\AppData\Roaming\3Di\QGIS3\profiles\default\python\plugins\lizard_qgis_plugin\deps\threedi_mi_utils-0.1.2-py3-none-any.whl
.
C:\Users\leendert.vanwolfswin\AppData\Roaming\3Di\QGIS3\profiles\default\python\plugins\threedi_models_and_simulations\deps\PyJWT-1.7.1-py2.py3-none-any.whl

hoanphungt commented 4 months ago

@leendertvanwolfswinkel hmm, I don't have this error as you got above and also the video doesn't work. I just merged master back to the water-quality branch, could you pull the latest commit in the water-quality branch and test it again?

leendertvanwolfswinkel commented 4 months ago

@hoanphungt I still have the same error. Did you try with the same model & simulation template?

hoanphungt commented 4 months ago

@leendertvanwolfswinkel Can you make a screen recording? The last one you made didn't work.

nens / threedi-api-qgis-client

Simulation wizard: Add substances page #534