-
Notifications
You must be signed in to change notification settings - Fork 3
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
🔧 Implement baseline scenario day selection for initial scenar… #369
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much, for a first time visitor this already works as inteded. This sadly interferes with the persistence, since the state for revisiting users gets overridden each time.
Can you try to prefer the state from the store and only do this when a user visits ESID for the first time?
@JonasGilg My bad. I completely overlooked the persistance part. Is it okay to set the activeScenarios directly in the DataSelectionSlice initial setting (activeScenarios: [0,1])? |
Yes, both should be possible. As long as it works. I don't see a problem with this solution. Just be sure, that the site doesn't break if for some reason no scenario is available. |
Test Results68 tests +1 67 ✅ ±0 15s ⏱️ -1s For more details on these failures, see this check. Results for commit 29ba089. ± Comparison against base commit d51f9b4. ♻️ This comment has been updated with latest results. |
07e8add
to
e709827
Compare
@JonasGilg I've implemented your suggestions. I added a userVisited flag in the UserPreference slice to track initial user visits. The baseline scenario card is now initialized by default in activeScenarios in the DataSelection slice. The useEffect in DataCardList component selects the baseline scenario if available. I reverted earlier changes to the DataCardList component as they are not required anymore. Do you have any suggestions for checking the user's initial visit without using the redux store? I tried a few approaches but couldn't find a suitable solution other than this. |
29ba089
to
d51f9b4
Compare
@JonasGilg I had some issues with merging this Pull Request as it was stale after the update of the application to use VVAFER components. I ran into some trouble while merging with the develop with the stale PR. I have made the changes now could you please go through it and review the changes? |
The implementation with the redux store is good. It is easily reusable for other components, if needed. |
…io list
Description
On initial load we had only case data card activated by default. Made changes to also activate the first scenario card (assuming it to be baseline scenario). The initial date would be the end date available for the particular scenario. Also added startDate and endDate calculating function in DataCardList component for reusability.
Related Issues
#364
Design Decisions
Performance & Quality
Checklist
I, the author of this PR checked the following requirements for good software quality:
I, the reviewer checked the following things: