Transaction notifications were not legible in some mobile views. Function added to notifications.js to detect whether user is on mobile or desktop, and sets the notification's style accordingly. On mobile, notification now has margin-top: 10% to make it legible if the device is a mobile. Related to task T1889
Details
Details
- Reviewers
bytesofman - Group Reviewers
Restricted Project - Commits
- rABC1de6e2c93142: [Cashtab] Change notification position on mobile
cd web/cashtab && npm start
Inspect and toggle device to mobile in chrome
send XEC and ensure notification is pushed lower in the viewport
create token and ensure notification is pushed lower in viewport
submit token icon and ensure notification is pushed lower in viewport
send token and ensure notification is pushed lower in viewport
receive XEC and ensure notification is pushed lower in viewport
receive eToken and ensure notification is pushed lower in viewport
sign message and ensure notification is pushed lower in viewport
force error and ensure notification is pushed lower in viewport
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- notification-placement-mobile
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 17566 Build 34958: Build Diff cashtab-tests Build 34957: arc lint + arc unit
Event Timeline
| web/cashtab/src/components/Common/Notifications.js | ||
|---|---|---|
| 12 | We don't want this to be a globally defined variable for this file, delete this line | |
| 14 | I think this function name could be improved, say getDeviceNotificationStyle() | |
| 31 | This should read const notificationStyle = identifyDeviceAndSetNotificationStyle() in all instances | |
Comment Actions
Removed globally defined variable, updated function name and changed function call per review feedback.
Comment Actions
1st level review is green - tested all ok on iOS and Android devices.
Linking D10667 here as a reminder to implement these visual enhancements for the NFT notifications as well down the track.