Fix and enhance comment editor monospace toggle #36181
Conversation
GiteaBot
added
the
lgtm/need 2
|
I think this could be expanded to also apply the setting in other open tabs by leveraging the https://developer.mozilla.org/en-US/docs/Web/API/Window/storage_event I guess a small storage helper module is needed to abstract this out of the file and be useful for all localStorage-backed variables. |
silverwind
added
type/enhancement
|
I've added the bare minimum to get the setting working across tabs too. There's probably an opportunity to abstract into some |
silverwind
changed the title
|
|
GiteaBot
added
lgtm/need 1
| /** Get a setting from localStorage */ | ||
| export function getLocalStorageSetting(key: string) { | ||
| return localStorage?.getItem(key); | ||
| } | ||
|
|
||
| /** Set a setting in localStorage */ | ||
| export function setLocalStorageSetting(key: string, value: string) { | ||
| return localStorage?.setItem(key, value); | ||
| } |
There was a problem hiding this comment.
How do these two wrapper functions help?
Why not just write localStorage.getItem()?
There was a problem hiding this comment.
The wrappers are in preparation to synchronize these settings with the server, e.g. the current settings would be rendered into <head> and it would trigger a POST when a setting changes. For non-logged-in users, localStorage will be continued to be used. So to support both cases, these wrappers are needed.
Maybe the functions should have a better name like getUserSetting.
There was a problem hiding this comment.
Thought this addLocalStorageChangeListener could not be implemented with server-backed storage without a websocket, I suppose. Maybe I should remove it.
| } | ||
|
|
||
| /** Apply font to the provided or all textareas on the page and optionally save on localStorage */ | ||
| function applyMonospace(enabled: boolean, {textarea, save}: {textarea?: HTMLTextAreaElement, save?: boolean}) { |
There was a problem hiding this comment.
It should be a method of ComboMarkdownEditor
And to change others, getComboMarkdownEditor and call their instance's method.
There was a problem hiding this comment.
Might be possible if there's a way to get references to all current ComboMarkdownEditor on the page.
There was a problem hiding this comment.
Use getComboMarkdownEditor for the related elements to get all current ComboMarkdownEditor instances
4 participants
Fixes: #36175