Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable user to navigate within the pages of an experiment via navigation stepper #423

Conversation

Steffengreiner
Copy link
Contributor

@Steffengreiner Steffengreiner commented Nov 7, 2023

What was changed

1.) Introduce dedicated AppLayout ExperimentMainLayout for all pages within an Experiment
This AppLayout hosts the ProjectSideNavigationComponent in its Drawer Section and the `ExperimentNavigationComponent' in its Navbar section.

2.) Adapt Routing to the SampleMainComponent
from projects/:projectId?/samplesto projects/:projectId?/experiments/:experimentId?/samples

Since the component is planned to show the batch information within an experiment instead of all experiment information within a project. This is also necessary so the ExperimentNavigationComponent knows the pages of which experiment it should route to.

3.) Remove ProjectNavigationBarComponent
No longer necessary since it's replaced with 'ExperimentNavigationComponent', this also means that the CSS for all main components was adapted to remove the space within the pages CSS layout grid allocated to the ProjectNavigationBarComponent.

4.) Minor adaptions to the component routing
We can now expect an ExperimentId to be present when navigating between the pages of an experiment, therefore the routing in all components and considering the beforeEnterEvents will throw an exception if no valid ExperimentId was required.

Technical Limitations and Bugs

1.) Active Experiment within the SideNav component is not highlighted correctly if the user switches to the ExperimentInformationPage within the experiment via the ExperimentNavigationComponent:

Bildschirmaufnahme.2023-11-07.um.14.19.58.mov

This seems to be a bug within the SideNavComponent which will hopefully be addressed by Vaadin in the future.
Sadly currently the SideNav component also does not provide the API to set an active Item directly so we can't do an easy workaround for now.

2.) Selecting an experiment via the tab within the SampleGrid does not update the active experiment in the SideNavComponent

Bildschirmaufnahme.2023-11-07.um.14.19.34.mov

This was not implemented since we plan to replace the experiment tabs within the sample grid with tabs showing batch information instead.

Bildschirmfoto 2023-11-07 um 14 12 43

@Steffengreiner Steffengreiner marked this pull request as ready for review November 7, 2023 14:03
@Steffengreiner Steffengreiner requested a review from a team as a code owner November 7, 2023 14:03
log.debug(String.format(
"New instance for ExperimentInformationMain (#%s) created with ProjectNavigationBar Component (#%s), ExperimentMain component (#%s) and ExperimentSupport component (#%s)",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use variables to reference class names, since they are mutable and can change. getClass().getName() sth like that

Copy link
Contributor Author

@Steffengreiner Steffengreiner Nov 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh good point! I took the liberty and adapted all class instantiation logs in this commit in a similiar way.

sven1103
sven1103 previously approved these changes Nov 10, 2023
Copy link
Contributor

@sven1103 sven1103 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job !

Copy link

sonarcloud bot commented Nov 10, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@Steffengreiner Steffengreiner merged commit fe77160 into development Nov 10, 2023
5 checks passed
@Steffengreiner Steffengreiner deleted the feature/dm-1002-provide-navigation-inside-an-experiment branch November 10, 2023 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants