Details
- Reviewers
bartek tomek inka - Commits
- rCOMM0023c11bb001: [native] Use new Alert wrapper
Test on iOS (setting alert theme is only supported on this platform) and on Android (to check if nothing breaks).
Prior to testing, make sure what theme you have set on iOS device. Then set app theme opposite to theme set on device. This will make sure, that Alerts use app theme rather than system theme.
Testing each location where the Alert import was replaced can be time-intensive, therefore only selected areas have been inspected to determine if the Alert function operates as expected:
- Login panel -> invalid username, empty password
- Edit message -> cancel editing
- Edit password -> not matching passwords, incorrect pasword
- Logout -> keep login info in chain alert
- Delete account panel -> invalid password
- Chat -> leave chat
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Please note that the only place where we still use Alert, imported from react-native, is in redux-setup.js. utils/alert.js uses the redux store to determine which theme to use. Attempting to use the alert wrapper prior to store initialization results in a You may not call store.getState() while the reducer is executing error. Although we could bypass this issue by using store.subscribe(variableUpdateCallback), the following reasons underscore why it's not an optimal approach:
- The variable updates with every redux store update, potentially leading to performance issues.
- We face a require cycle problem.
You can emulate second reason by changing import in redux-setup.js to our alert wrapper and replacing getCurrentTheme in utils/alert.js with something like this:
// ... let currentTheme = defaultGlobalThemeInfo.activeTheme; store.subscribe(() => { const nextTheme = store.getState().globalThemeInfo.activeTheme || defaultGlobalThemeInfo.activeTheme; if (nextTheme !== currentTheme) { currentTheme = nextTheme; } }); // Remember to swap all `getCurrentTheme()` occurrences to `currentTheme` // ...
One additional thing to check might be Android - even though the theme doesn't apply there, it is worth checking if nothing breaks.