-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[No QA] Allow mapbox token to stop listening to all listeners and add a network listener #25408
Conversation
@tgolen thank you so much for helping me with this PR. I have one question. Should we use a custom hook with |
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.
but otherwise the PR looks good. We can convert this to a custom hook as a clean up issue if necessary. we just have to be careful to call stop
I think we want to do something other than a custom hook for the mapbox token. If there are multiple |
Reviewer Checklist
Screenshots/VideosThis code is tested in this PR. WebN/A Mobile Web - ChromeN/A Mobile Web - SafariN/A DesktopN/A iOSN/A AndroidN/A |
I gonna merge this hope you don't mind @Ollyws 😓 |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/hayata-suenaga in version: 1.3.56-0 🚀
|
🚀 Deployed to production by https://github.com/roryabraham in version: 1.3.56-24 🚀
|
cc @neil-marcellini
Details
This PR adds the ability to stop all the listeners for the mapbox token and let's the token expire naturally. This is used for when discount requests unmount.
It also adds a listener for the network connection so that if the token has expired, it will fetch a new one.
Fixed Issues
$ #24612
Tests
Note: this can only be tested in web/desktop by an engineer that can modify the local code
USE THIS DIFF FOR TESTING
REFRESH_INTERVAL
to be1000
only[MapboxToken]
setTimeout(MapboxToken.stop, 2000);
fromAuthScreens.js
Force offline
Force offline
Offline tests
Included above
QA Steps
None
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Web
Mobile Web - Chrome
Note: this can only be tested on web
Mobile Web - Safari
Note: this can only be tested on web
Desktop
Note: this can only be tested on web
iOS
Note: this can only be tested on web
Android
Note: this can only be tested on web