search

dianna-ai / dianna

Deep Insight And Neural Network Analysis
https://dianna.readthedocs.io
Apache License 2.0
48 stars 13 forks source link

817 dashboard move menu #830

Closed laurasootes closed 3 months ago

laurasootes commented 4 months ago

This PR moves the menu from the left to the top of the page. This deviates from streamlits defaults, and requires the use of dynamic page imports. Also the ‘original menu’ has to be hidden in the settings.

These adjustments cause the changed parameters in each page to be conserved between pages. To solve this I added page-specific keys to the variable elements, which are reset when you switch pages.

Also added icons to the menu items :)

Solves #817

laurasootes commented 3 months ago

@SarahAlidoost thanks for your review! I implemented your code suggestions and the linting now passes.

I looked at the error you mention for the dashboard titles. I do not know why they would arise. Also I do not get them locally and don’t see them in the logs in the github actions either so I am a bit puzzled. Could you maybe try yo run them again?

I added your comment for the pages section on the home page to #816, because I think there will be some more discussion on what should and should not be on the home page

SarahAlidoost commented 3 months ago

@SarahAlidoost thanks for your review! I implemented your code suggestions and the linting now passes.

I looked at the error you mention for the dashboard titles. I do not know why they would arise. Also I do not get them locally and don’t see them in the logs in the github actions either so I am a bit puzzled. Could you maybe try yo run them again?

You are right. Perhaps something related to running the tests locally on my side. I ran the dashboard test as pytest -v -m dashboard --dashboard on the main branch too and got the same error. Somehow the titles of pages are for example "Text · Streamlit" instead "Text" on my side :smile: . As you don't get the errors locally, I approved this PR. The failed GitHub actions are due to issue #837 .