-
Notifications
You must be signed in to change notification settings - Fork 541
[core-v2.3.1-alpha.4, react-v2.2.1-alpha.4, vue-v2.1.1-alpha.3] : Update - Decouple Notify and AC positioning #1098
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
fd7e367
5fefd3b
00e8ef5
fa2cc09
74a1a25
e2c3968
1a60785
2fcd526
3a91656
dfa0627
7121f58
a3e52f9
603cc6c
32a4ec0
1c462f3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,7 +46,7 @@ export interface InitOptions { | |
| /** | ||
| * Transaction notification options | ||
| */ | ||
| notify?: Partial<NotifyOptions> | ||
| notify?: Partial<NotifyOptions> | Partial<Notify> | ||
| } | ||
|
|
||
| export interface OnboardAPI { | ||
|
|
@@ -63,7 +63,7 @@ export interface OnboardAPI { | |
| interface ExposedActions { | ||
| setWalletModules: (wallets: WalletInit[]) => void | ||
| setLocale: (locale: string) => void | ||
| updateNotify: (update: Partial<NotifyOptions>) => void | ||
| updateNotify: (update: Partial<Notify>) => void | ||
| customNotification: ( | ||
| updatedNotification: CustomNotification | ||
| ) => { | ||
|
|
@@ -141,7 +141,7 @@ export interface AppState { | |
| wallets: WalletState[] | ||
| accountCenter: AccountCenter | ||
| locale: Locale | ||
| notify: NotifyOptions | ||
| notify: Notify | ||
| notifications: Notification[] | ||
| } | ||
|
|
||
|
|
@@ -156,11 +156,15 @@ export type Locale = string | |
| export type i18nOptions = Record<Locale, i18n> | ||
| export type i18n = typeof en | ||
|
|
||
| export type AccountCenterPosition = | ||
| | 'topRight' | ||
| | 'bottomRight' | ||
| | 'bottomLeft' | ||
| | 'topLeft' | ||
| export type CommonPositions = | ||
taylorjdawson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| | 'topRight' | ||
| | 'bottomRight' | ||
| | 'bottomLeft' | ||
| | 'topLeft' | ||
|
|
||
| export type AccountCenterPosition = CommonPositions | ||
|
|
||
| export type NotificationPosition = CommonPositions | ||
|
|
||
| export type AccountCenter = { | ||
| enabled: boolean | ||
|
|
@@ -174,7 +178,7 @@ export type AccountCenterOptions = { | |
| mobile: Omit<AccountCenter, 'expanded'> | ||
| } | ||
|
|
||
| export type NotifyOptions = { | ||
| export type Notify = { | ||
| /** | ||
| * Defines whether whether to subscribe to transaction events or not | ||
| * default: true | ||
|
|
@@ -189,6 +193,17 @@ export type NotifyOptions = { | |
| transactionHandler: ( | ||
| event: EthereumTransactionData | ||
| ) => TransactionHandlerReturn | ||
| /** | ||
| * Position of notifications that defaults to the same position as the | ||
| * Account Center (if enabled) of the top right if AC is disabled | ||
| * and notifications are enabled (enabled by default with API key) | ||
| */ | ||
| position?: NotificationPosition | ||
| } | ||
|
|
||
| export type NotifyOptions = { | ||
| desktop: Notify | ||
| mobile: Notify | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Adamj1232 Could we possible discuss naming conventions here?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @taylorjdawson I see what you are saying, I was following the pattern we have used for AccountCenter where
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay no worries, is there a way to make the name more descriptive? When I saw the |
||
| } | ||
|
|
||
| export type Notification = { | ||
|
|
@@ -279,7 +294,7 @@ export type SetLocaleAction = { | |
|
|
||
| export type UpdateNotifyAction = { | ||
| type: 'update_notify' | ||
| payload: Partial<NotifyOptions> | ||
| payload: Partial<Notify> | ||
| } | ||
|
|
||
| export type AddNotificationAction = { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.