Added: in page notification options#127
Conversation
Changed: browser sync storage as there is a limit to how manny packages you can send to the cloud (this is done with a timestamp counter) > 10 curently. if threasshold is reached it will delay send and send it one second later. further "adds" are added to the same "batch" and time is reset by 1 sec Changed so that it wont .set of value already equal to .get
| try { | ||
| await browser.storage.sync.set({ [key]: toStore }); | ||
| console.log(`BrowserSyncStorage.set: ${key} = ${toStore}`); | ||
| if (currentValue != value) { |
There was a problem hiding this comment.
If the value is usually saved as a JSON.stringify'd value, don't you need to compare what's in the storage with a JSON.stringify'd value instead of the value of unknown type?
There was a problem hiding this comment.
The .get do JSON.parse before returning value so it should be back to the "real" value, and value is the same ? or did i miss something?
There was a problem hiding this comment.
The opposite might be better where you
const newValue = JSON.stringify(value);and then check if currentValue != newValue or something like that.
public/_locales/en/messages.json
Outdated
| } | ||
| }, | ||
| "DISMISS_TIME_LABEL": { | ||
| "message": "Dismiss time, $houers$ houers", |
schang1146
left a comment
schang1146
left a comment
There was a problem hiding this comment.
Left some comments, started to fade and couldn't review:
- preferences.ts
- browser-sync-storage.ts
- Inspagenotification.tsx
| } | ||
|
|
||
| export interface IInPageNotificationPageMenu { | ||
| pageReferance: React.RefObject<HTMLParagraphElement | null>; |
There was a problem hiding this comment.
Spelling
| pageReferance: React.RefObject<HTMLParagraphElement | null>; | |
| pageReference: React.RefObject<HTMLParagraphElement | null>; |
There was a problem hiding this comment.
Please look for other references to "referance".
src/storage/istorage-backend.ts
Outdated
|
|
||
| /* | ||
| export interface IStorageSyncBackend { | ||
| buffer: Map<string, unknown>; | ||
| removals: string[]; | ||
| flushTimeout: NodeJS.Timeout | null; | ||
|
|
||
| set(key: string, value: unknown): Promise<void>; | ||
| get(key: string): Promise<unknown>; | ||
| remove?(key: string): Promise<void>; | ||
| scheduleSync(delay: number): void; | ||
| } | ||
| */ |
There was a problem hiding this comment.
Is this going to be used in the future? If not, please remove unnecessary commented out code.
| /* | |
| export interface IStorageSyncBackend { | |
| buffer: Map<string, unknown>; | |
| removals: string[]; | |
| flushTimeout: NodeJS.Timeout | null; | |
| set(key: string, value: unknown): Promise<void>; | |
| get(key: string): Promise<unknown>; | |
| remove?(key: string): Promise<void>; | |
| scheduleSync(delay: number): void; | |
| } | |
| */ |
src/ui/options/Options.module.css
Outdated
| datalist { | ||
| display: flex; | ||
| justify-content: space-between; | ||
| writing-mode: lr; | ||
| width: 100%; | ||
| padding-left: 8px; | ||
| padding-right: 5px; | ||
| } |
There was a problem hiding this comment.
Add common class name that can go on both <datalist> elements to share the styling.
Trying to avoid targeting a HTML element in case we want to use <datalist> in a different way elsewhere.
src/ui/options/Options.module.css
Outdated
| transform: translateX(22px); | ||
| } | ||
|
|
||
| .slidecontainer { |
There was a problem hiding this comment.
| .slidecontainer { | |
| .sliderContainer { |
| console.log(`BrowserSyncStorage.set: ${key} = ${toStore}`); | ||
| if (currentValue != value) { | ||
| const toStore = JSON.stringify(value); | ||
| if (this.checkQue()) { |
There was a problem hiding this comment.
Spelling
| if (this.checkQue()) { | |
| if (this.checkQueue()) { |
3 participants
Changed: browser sync storage as there is a limit to how manny packages you can send to the cloud (this is done with a timestamp counter) > 10 curently. if threasshold is reached it will delay send and send it one second later. further "adds" are added to the same "batch" and time is reset by 1 sec Changed so that it wont .set of value already equal to .get
mainbranch.npm testnpm run format