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

Fix unwanted welcomeModal display #11449

Conversation

ozer550
Copy link
Member

@ozer550 ozer550 commented Oct 23, 2023

Summary

  • Fix Incorrect welcomeModal display for guest users in learn tab.
  • Change session storage to local storage for welcomeDismissalKey.

Screen shots

Before:
image

After:
image

References

closes #11202

Reviewer guidance

  • Import a channel.
  • Sign out explore as guest.
  • Go to learn tab .
  • welcomeModal does not appear.

Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

PR process

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@github-actions github-actions bot added APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) APP: Learn Re: Learn App (content, quizzes, lessons, etc.) SIZE: small labels Oct 23, 2023
@pcenov
Copy link
Member

pcenov commented Oct 23, 2023

Hi @ozer550 and @marcellamaki - I confirm that the original issue as described in #11202 is fixed now however if I delete all browser cookies and sign in again as a super admin I'm still seeing the rogue modal while already having imported channels. So this will keep happening for admin users every time they sign in in a new browser.

@rtibbles
Copy link
Member

@pcenov this is definitely not a regression and is how it would have been previously, I am fairly sure - we are not tracking the dismissal in anything other than browser state, so clearing the application data will also clear the fact that the modal has been dismissed.

Copy link
Member

@rtibbles rtibbles left a comment

Choose a reason for hiding this comment

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

Code changes look correct to me.

Copy link
Contributor

@thanksameeelian thanksameeelian left a comment

Choose a reason for hiding this comment

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

this is looking good to me as well!

the change from sessionStorage to localStorage makes sense & i checked all the places i'd experienced the modal popping up at inappropriate times and was unable to replicate the previous buggy behavior! (aside from the behavior @pcenov pointed out, which as @rtibbles noted is not a regression, but more a symptom of the way we're tracking users' interactions with the modal).

thanks for tackling this @ozer550!!

@pcenov
Copy link
Member

pcenov commented Oct 24, 2023

@rtibbles I'm not claiming it's a regression, I am pointing out that we are partially fixing a preexisting issue. The problem here is that since we are not tracking the dismissal in anything other than browser state the 'Welcome to Kolibri!' modal will show on the Library page and on Device > Channels regardless of whether the user has already imported channels or not. So it works correctly only if I am using the same browser. I can file it as a separate issue if there's anything that can be done?

Library:

2023-10-24_10-01-38.mp4

Device > Channels:

2023-10-24_10-08-29

@marcellamaki
Copy link
Member

Hi @pcenov - I think filing this as a separate issue would be great! It does make sense that we might want to add some checking about whether or not there are channels imported. Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) APP: Learn Re: Learn App (content, quizzes, lessons, etc.) SIZE: small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Nothing in your library" pop up displays for guest users with library content
5 participants