Make Periodic Table widget toggleable

[CasperWA]

Closes #201.

See on binder.

Wrap the checkbox and periodic table widget in a VBox container and add a button to change toggle the visibility layout parameter between visible and hidden.

A suggestion from my part would be to minimize the button and/or move it to the left side, but it might also be all right as is?

[codecov]

Codecov Report

Merging #203 into develop will decrease coverage by 0.29%.
The diff coverage is 18.51%.

@@             Coverage Diff             @@
##           develop     #203      +/-   ##
===========================================
- Coverage    35.96%   35.67%   -0.30%     
===========================================
  Files           17       17              
  Lines         2138     2161      +23     
===========================================
+ Hits           769      771       +2     
- Misses        1369     1390      +21     

Flag	Coverage Δ
optimade-client	35.67% <18.51%> (-0.30%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
optimade_client/query_filter.py	12.59% <0.00%> (-0.35%)	⬇️
optimade_client/summary.py	28.70% <0.00%> (-0.14%)	⬇️
optimade_client/subwidgets/periodic_table.py	36.95% <17.64%> (-13.05%)	⬇️
optimade_client/subwidgets/filter_inputs.py	27.09% <25.00%> (-0.37%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab7ea03...e0be053. Read the comment docs.

[csadorf]

Before I review this, the binder appears to be broken:

ModuleNotFoundError: No module named 'appmode'
Removing intermediate container 24295b9f96f2
The command '/bin/sh -c ./binder/postBuild' returned a non-zero code: 1

[CasperWA]

Before I review this, the binder appears to be broken:

ModuleNotFoundError: No module named 'appmode'
Removing intermediate container 24295b9f96f2
The command '/bin/sh -c ./binder/postBuild' returned a non-zero code: 1

Fixed in #205. The link seems to work now.
To test the toggle button, simply choose a database to enable the button.

[csadorf]

If the periodic table can only be toggled if a data base is selected that would defeat the purpose. So at least programmatically it should be possible to toggle the visibility independently. For this I suggest to add a trait to the widget and connect the toggle button internally with that trait. See also in-code comments for details.

Two additional points which are out of scope for this PR:

1. Whether the table is available/visible or not, I would expect to be able to always also enter elements "manually" into a text field which would be synchronized with the table.
2. It seems to me that there is no clear explanation as to what the various "selection states" represent as part of the query. Clicking elements multiple times makes them change color, and I assume based on the color that it means "included/excluded", but it is not immediately obvious that this is the meaning in the context of the query and it would be completely impossible to discern for a person with red/green color-blindness.

[CasperWA]

If the periodic table can only be toggled if a data base is selected that would defeat the purpose. So at least programmatically it should be possible to toggle the visibility independently. For this I suggest to add a trait to the widget and connect the toggle button internally with that trait. See also in-code comments for details.

If embedded=True is passed to the OptimadeQueryFilterWidget widget the Periodic Table widget will be toggled "off"/hidden by default.
The current UX is to keep all filter inputs disabled until a database has been chosen. This is to guide the user to this point first.
Now to the issue here: Whether one can show or hide the disabled Periodic Table widget I thought to fall under the same umbrella, and the initial configuration can be set up using the embedded parameter. But I have no issue making the toggle button special in this regard. So if you think it seems more natural to be able to toggle the input, no matter if the shown/hidden widget is disabled or not, then I'll implement that here.

1. Whether the table is available/visible or not, I would expect to be able to always also enter elements "manually" into a text field which would be synchronized with the table.

This is a conscious UX choice at this point (to only use the Periodic Table widget).
However, I will raise this point as an issue, but I don't expect this alternative to be implemented in the near future.

2. It seems to me that there is no clear explanation as to what the various "selection states" represent as part of the query. Clicking elements multiple times makes them change color, and I assume based on the color that it means "included/excluded", but it is not immediately obvious that this is the meaning in the context of the query and it would be completely impossible to discern for a person with red/green color-blindness.

This is already the subject of issue #160.
The color-blindness is indeed a valid concern I hadn't previously considered.

[csadorf]

If the periodic table can only be toggled if a data base is selected that would defeat the purpose. So at least programmatically it should be possible to toggle the visibility independently. For this I suggest to add a trait to the widget and connect the toggle button internally with that trait. See also in-code comments for details.

If embedded=True is passed to the OptimadeQueryFilterWidget widget the Periodic Table widget will be toggled "off"/hidden by default.
The current UX is to keep all filter inputs disabled until a database has been chosen. This is to guide the user to this point first.
Now to the issue here: Whether one can show or hide the disabled Periodic Table widget I thought to fall under the same umbrella, and the initial configuration can be set up using the embedded parameter. But I have no issue making the toggle button special in this regard. So if you think it seems more natural to be able to toggle the input, no matter if the shown/hidden widget is disabled or not, then I'll implement that here.

I think that the API I'm suggesting provides more control and flexibility, but as long as the widget is hidden by default, it's fine with me.

1. Whether the table is available/visible or not, I would expect to be able to always also enter elements "manually" into a text field which would be synchronized with the table.

This is a conscious UX choice at this point (to only use the Periodic Table widget).
However, I will raise this point as an issue, but I don't expect this alternative to be implemented in the near future.

It feels non-intuitive and unnecessarily restrictive to me, but again, out of scope for this PR anyways.

[csadorf]

@CasperWA Is this ready for another review?

Review has been addressed.

[CasperWA]

@csadorf I will rebase and merge some commits and merge. No need to re-approve.