-
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
Second attempt for #24306 #25496
Second attempt for #24306 #25496
Conversation
Making PR draft as this is not ready yet |
I assigned you @allroundexperts because you reviewed this PR. We since reverted that PR because of an issue on CI builds (which turned out to be a false alarm). anyway, this is the second attempt for the original PR. The only change I made since then is this line cc: @amyevans |
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.
@hayata-suenaga shouldn't we be seeing more of a diff here? I only see the testBuild
CI change 😕
@Julesssss so I created PR with the same branch from the PR that has been reverted. I don't know why there aren't more diffs here. Because I'll keep investigating this but do you have any idea why this might be the case? |
Closing this in favor of this PR |
Details
This PR does three things
Add scripts necessary to setup the environment to download Mapbox iOS and Android SDKs
You have to get credentials from Mapbox and add them to appropriate files to download Mapbox SDKs. Added scripts to streamline that process.
Configure the project for Mapbox SDKs
Mapbox SDKs requires configurations in
build.gradle
andPodfile
.Install
react-native-x-maps
which uses Mapbox mobile and web SDKs under the hood to display mapsAdded a map in the Distance Request page (where users can report expenses for automobile transportations).
Fixed Issues
$ #22703
PROPOSAL: N/A
Tests / QA Steps
Map test
For engineers, if you're building and testing this PR in the development environment, you first need to run
npm run configure-mapbox
to setup the development environment.Bash script test (This doesn't have to be QAed by the QA team)
Run
npm run configure-mapbox
for the project root.Check you see an output like below.
Don't enter anything for the secret token prompt and hit enter. Check you receive the following message
Run
npm run configure-mapbox
. Type something for the secret token prompt (it doesn't have to be the actual token). Note that you don't see what's you're typing. Check you see an output like below.Open
~/.netrc
. Check that you have an entry like the followingOpen
/.gradle/gradle.properties
. Check you have an entry like the following.Offline tests
If you open
DistanceRequest
page by following the test instruction for "Map test" while offline, you should not see the map.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
Mobile Web - Safari
Desktop
iOS
Android