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

Add mobile blocker page #348

Merged
merged 12 commits into from
Apr 10, 2020
Merged

Add mobile blocker page #348

merged 12 commits into from
Apr 10, 2020

Conversation

as-dr
Copy link
Contributor

@as-dr as-dr commented Apr 8, 2020

Closes #344

image

@as-dr as-dr requested a review from Schwartz10 April 8, 2020 15:43
@Schwartz10
Copy link
Contributor

@listenaddress a couple things left in here for you:

  1. add logic for isMobileOrTablet - i already found some code for detecting mobile, just need to find a good one for tablets.
  2. add tests for isMobileOrTablet
  3. Add tests for the useDesktopBrowser hook, you can stub 2 things here:
    a. you can stub the isMobileOrTablet as a jest mocked function, so it returns true
    b. you can stub next/router module so that you can assert that the replace function was called, passing /error/use-desktop-browser. I do something similar here: https://github.com/openworklabs/filecoin-web-wallet/blob/add/mobileblocker/lib/check-network.test.js#L10-L11 but you might need to use a different strategy, that uses spyOn for this (something like How to mock useRouter? vercel/next.js#7479 (comment))
  4. snapshot testing for the error pages could go in this PR, or we could open that in a separate follow-on PR

@Schwartz10 Schwartz10 changed the title Add mobile blocker page (WIP) Add mobile blocker page Apr 9, 2020
@listenaddress listenaddress changed the title (WIP) Add mobile blocker page Add mobile blocker page Apr 10, 2020
@listenaddress listenaddress merged commit 6d89113 into primary Apr 10, 2020
@listenaddress listenaddress deleted the add/mobileblocker branch April 10, 2020 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

block mobile
3 participants