feat: add config attribute to enable/disable contacts section in Settings screen, default as enabled #1374
Conversation
| @@ -167,6 +167,11 @@ const Settings: React.FC<SettingsProps> = ({ navigation }) => { | |||
| }, | |||
| ...(settings || []), | |||
| ] | |||
|
|
|||
| // Remove the Contact section from Setting per TOKENS.CONFIG | |||
| if ("true" === `${disableContactsInSettings}`) { | |||
There was a problem hiding this comment.
| if ("true" === `${disableContactsInSettings}`) { | |
| if (disableContactsInSettings) { |
There was a problem hiding this comment.
Will change it. The bad habit from writing plain JS, worry about entering other strings to make it to be true.
packages/legacy/app/ios/Podfile.lock
Outdated
There was a problem hiding this comment.
This change should not be included in this PR. Rather than run pod install you can use the command yarn ios:setup from the app folder to properly install pods without diverging from the set versions in the Gemfile.
There was a problem hiding this comment.
Thanks, that's a big help to me
There was a problem hiding this comment.
This diff should not be committed as it is a change specific to your local development environment, and will be different for different developers
|
@bryce-mcmath, The ios unit tests failed in build stage after restored the "Podfile.lock" and "project.pbxproj" files. The log shows the "IPHONEOS_DEPLOYMENT_TARGET = 11.0" defined in "project.pbxproj" not supported by the Pods specified devices range. |
bryce-mcmath
changed the title
So I don't see anything in your PR that would cause this error, the only change remaining is the EOF changes to the iOS files which you could remove and see if it helps. In the meantime I have rerun the action, hopefully it was just a one off. But yes, we should fix any build issues before merging |
packages/legacy/app/ios/Podfile.lock
Outdated
There was a problem hiding this comment.
There is still an EOF change to this file and the project.pbxproj file. It's unlikely, but perhaps the action will succeed if we make no unnecessary changes to these files, not even EOF. Other PRs in this repo seem to be passing so I don't know what else the cause could be
There was a problem hiding this comment.
Fixed, sorry for making this weird trouble. My IDE change those files after CTRL+S even I didn't enter anything.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1374 +/- ##
==========================================
+ Coverage 55.17% 55.73% +0.55%
==========================================
Files 219 220 +1
Lines 7737 7897 +160
Branches 2192 2229 +37
==========================================
+ Hits 4269 4401 +132
- Misses 3446 3473 +27
- Partials 22 23 +1 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Jian Wang <[email protected]>
…to remove the Contacts section in Settings rows, added test case for this feature Signed-off-by: Jian Wang <[email protected]>
…atement matching the boolean type Signed-off-by: Jian Wang <[email protected]>
…stream to fix the file EOF issue Signed-off-by: Jian Wang <[email protected]>
|
|
@jian4on If you make a connection without a goal code, the user is redirected to the Chat Screen and from there the user can navigate to the Contact Details. From there, the user could delete the contact which then after success the user is redirected to the Contact List Screen. If a wallet is configured to disable the contacts section, then it would not make sense to redirect the user to the Contact Screen here in the ContactDetails.tsx: // Ligne 155
const callSubmitRemove = useCallback(async () => {
try {
if (!(agent && connection)) {
return
}
if (historyEventsLogger.logConnectionRemoved) {
logHistoryRecord()
}
await agent.connections.deleteById(connection.id)
/* ================= */
// Where should the user be redirected here ?
navigation.navigate(Screens.Contacts)
// FIXME: This delay is a hack so that the toast doesn't appear until the modal is dismissed
await new Promise((resolve) => setTimeout(resolve, 1000))
Toast.show({
type: ToastType.Success,
text1: t('ContactDetails.ContactRemoved'),
})
} catch (err: unknown) {
const error = new BifoldError(t('Error.Title1037'), t('Error.Message1037'), (err as Error)?.message ?? err, 1037)
DeviceEventEmitter.emit(EventTypes.ERROR_ADDED, error)
}
}, [agent, connection, navigation, t, historyEventsLogger.logConnectionRemoved, logHistoryRecord]) |
|
I will look into this issue and discuss with the team, then back to you, thanks.
…________________________________
From: Filipe Santos ***@***.***>
Sent: Monday, January 6, 2025 3:05 PM
To: openwallet-foundation/bifold-wallet ***@***.***>
Cc: Wang, Jian (MPBSDP) ***@***.***>; Mention ***@***.***>
Subject: Re: [openwallet-foundation/bifold-wallet] feat: add config attribute to enable/disable contacts section in Settings screen, default as enabled (PR #1374)
CAUTION -- EXTERNAL E-MAIL - Do not click links or open attachments unless you recognize the sender.
@jian4on<https://github.com/jian4on> If you make a connection without a goal code, the user is redirected to the Chat Screen and from there the user can navigate to the Contact Details. From there, the user could delete the contact which then after success the user is redirected to the Contact List Screen. If a wallet is configured to disable the contacts section, then it would not make sense to redirect the user to the Contact Screen here in the ContactDetails.tsx:
// Ligne 155
const callSubmitRemove = useCallback(async () => {
try {
if (!(agent && connection)) {
return
}
if (historyEventsLogger.logConnectionRemoved) {
logHistoryRecord()
}
await agent.connections.deleteById(connection.id)
/* ================= */
// Where should the user be redirected here ?
navigation.navigate(Screens.Contacts)
// FIXME: This delay is a hack so that the toast doesn't appear until the modal is dismissed
await new Promise((resolve) => setTimeout(resolve, 1000))
Toast.show({
type: ToastType.Success,
text1: t('ContactDetails.ContactRemoved'),
})
} catch (err: unknown) {
const error = new BifoldError(t('Error.Title1037'), t('Error.Message1037'), (err as Error)?.message ?? err, 1037)
DeviceEventEmitter.emit(EventTypes.ERROR_ADDED, error)
}
}, [agent, connection, navigation, t, historyEventsLogger.logConnectionRemoved, logHistoryRecord])
cc @bryce-mcmath<https://github.com/bryce-mcmath>
—
Reply to this email directly, view it on GitHub<#1374 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AXA4QNPRCNPUHTSFNHT5IDT2JLOXVAVCNFSM6AAAAABUEDXVRKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNZTHAZTAMJUHA>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
@fc-santos Thanks for pointing this out. Note that the intent of this change is remove contacts as a first order concept from the settings menu. It is not to disable contacts in the wallet. When using OID4VCI the current contacts screen is not useful to the user as it will always be empty. At some point contacts will need to be revisited as OID4VCI also has a concept of contacts. This will need some UX discussions. |
Summary of Changes
Added a new boolean field 'disableContactsInSettings' in Config, default value is false (Enabled)
Screenshots, videos, or gifs
N/A
Breaking change guide
N/A
Related Issues
N/A
Pull Request Checklist
Tick all boxes below to demonstrate that you have completed the respective task. If the item does not apply to your this PR check it anyway to make it apparent that there's nothing to do.
Signed-off-byline (we use the DCO GitHub app to enforce this)If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!
Pro Tip 🤓