Fetch/Display Mobile App QR Code #880
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
joeyorlando
force-pushed
the
jorlando/mobile-app-qr-code
branch
9 times, most recently
from
b6d1377 to
a012484
Compare
joeyorlando
changed the title
teodosii
force-pushed
the
jorlando/mobile-app-qr-code
branch
from
67c900f to
a3df014
Compare
joeyorlando
force-pushed
the
jorlando/mobile-app-qr-code
branch
from
6e6209d to
10faa57
Compare
joeyorlando
commented
Nov 28, 2022
| onClick: () => void; | ||
| }; | ||
|
|
||
| // TODO: right now this shows a confirmation pop-up modal on top of the user settings modal, do we want to maybe change this? |
There was a problem hiding this comment.
@teodosii UX related question to point out
joeyorlando
commented
Nov 28, 2022
| {activeTab === UserSettingsTab.MobileAppVerification && ( | ||
| <MobileAppVerification userPk={id} phone={storeUser.unverified_phone_number || '+'} /> | ||
| )} | ||
| {/* TODO: we should probably hide this tab when a user (ie. Admin) is viewing the user settings for another user. Would it make sense for an Admin to be able to link their mobile app to another user's profile */} |
There was a problem hiding this comment.
general question/comment. should we hide this tab when a user is viewing the settings for another user? I don't think it makes sense to allow user's to link their own mobile app to the profile of another user. Thoughts?
vstpme
approved these changes
Nov 28, 2022
joeyorlando
commented
Nov 28, 2022
| @action | ||
| getCurrentUser() { | ||
| return this.items[this.currentUserPk as User['pk']]; | ||
| } |
There was a problem hiding this comment.
we already have a UserStore.currentUser getter.. this didn't seem to be referenced anywhere in the codebase (nor in oncall-private), I think it's safe to remove
joeyorlando
force-pushed
the
jorlando/mobile-app-qr-code
branch
from
5585ae2 to
5a4fc90
Compare
|
@teodosii there is still some minor styling polishing I believe, but I'll go ahead and merge this and we can follow up in a subsequent PR with that (+ address the two questions I had) |
brojd
pushed a commit
that referenced
this pull request
Sep 18, 2024
Fetch/Display Mobile App QR Code
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does:
Adds support for fetching/displaying QR code that will (eventually) be used to link the mobile app to your OnCall instance.
https://www.loom.com/share/aa12bfd7f5b846459f5976161d4550ad
Checklist
CHANGELOG.mdupdated