In the process of optimizing notifications it became apparent that getDeviceNotificationStyle was a fix for a legacy issue where notifications were clipping the top frame of the browser on mobile devices. This is no longer required as the top margin is now adequate without the need to set mobile specific marginTop values.
Details
Details
- Reviewers
bytesofman - Group Reviewers
Restricted Project - Commits
- rABCec4f96193676: [Cashtab] Deprecate getDeviceNotificationStyle
npm test
npm start
Trigger the following notifications across web, extension and mobile (iOS Safari and android chrome):
- Send/Receive XEC
- Send/Receive/Burn eToken
- Create eToken and icon submission
- Message signed / copied
- Message/signature verified
- Message/signature not verified
- Register alias
- Error
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- depGetDeviceNotif
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 26777 Build 53122: Build Diff cashtab-tests Build 53121: arc lint + arc unit
Event Timeline
Comment Actions
one of the bigger style changes here is that links are now blue in the txs.
imo acceptable as the notifications looked bad before and continue to look bad. something to improve in another diff.
green with nit -- rebase to latest master & version bump.
Comment Actions
In the future please include some before and after screenshots.
we are not practically able to test every one of these notifications on every single browser and device. it's also not worth the time to do something like this manually, especially on what is effectively a maintenance diff.
either we have integration tests that catch the important stuff automatically, like the content that renders inside a component. Or, if it's css only, screenshots + test for responsiveness is ok.