Settings overhaul #275
Settings overhaul #275
Conversation
maxpoulin64
added
the
Type: Feature
This redoes pretty much everything related to client-side settings and unties them from the DOM. Most of the action happens in the `Settings` class in preparation for a future client-side API, which makes listening to setting changes a lot easier. Setting handlers never touches or sees the DOM. It also makes the binding glue between the settings and the DOM work the other way, so invisible settings are possible and can be tied to multiple controls or sources at once. This could be used later, for example, by a `/set` command or for any other reason. Communication with the server is also made separate for good measures, so the settings are usable without any active socket. A similar API is exposed on the server through `client.settings`, with the same separation of concerns, and will update all clients. These settings can be further used by the server to save preferences server-side, such as an highlight word list, auto away on/off, and many other things.
| }); | ||
|
|
||
| client.settings.bindToSocket(socket); |
There was a problem hiding this comment.
This shouldn't be bound in public mode.
There was a problem hiding this comment.
Actually yes. It's not currently needed, but I prefer having the functionality than it being a limitation later on. As I mentioned in the PR text itself, I'm aiming for API-ready code so I try to avoid putting arbitrary limitations. There are a few limited cases in which having the server aware of some settings could be useful. It's not used strictly for settings persistance, but also just for the server to share settings in general. I'm thinking /ignore support, auto away and others.
There's a (one-time) check here to not persist settings in public mode. I'm using client.name so if we eventually allow both public and private mode at the same time, it's ready.
There was a problem hiding this comment.
For mixed public/private mode we should have some better check other than client.name then.
|
We probably want to namespace the settings. As settings like hiding joins, hiding quits, notifyAllMessages ideally would be set per channel. So the current settings window would be |
| } | ||
|
|
||
| window.localStorage.setItem("settings", JSON.stringify(options)); | ||
| settings.element.find("input[name^='setting:'], textarea[name^='setting:']").each(function() { |
There was a problem hiding this comment.
Why not just use event delegation?
There was a problem hiding this comment.
I could but since this is a two-way sync, I'd need to keep querying and checking the type of element for every setting change. I thought it was clearer that way, especially since we don't have that many boxes to bind.
|
Just tested this locally, works fine, great work there! Only suggestion I have is to enable synchronization by default. |
|
|
||
| <div class="col-sm-12"> | ||
| <h2> | ||
| Custom Stylesheet |
There was a problem hiding this comment.
Make this a label and target it at the textarea please.
|
What's the deal with this? Are we planning on getting this updated to master? I think this is a sorely missed feature that would be fantastic to get in. |
|
I will close this PR for now. I've moved the |
This redoes pretty much everything related to client-side settings and unties them from the DOM. Most of the action happens in the
Settingsclass in preparation for a future client-side API, which makes listening to setting changes a lot easier. Setting handlers never touches or sees the DOM.It also makes the binding glue between the settings and the DOM work the other way, so invisible settings are possible and can be tied to multiple controls or sources at once. This could be used later, for example, by a
/setcommand or for any other reason.Communication with the server is also made separate for good measures, so the settings are usable without any active socket.
A similar API is exposed on the server through
client.settings, with the same separation of concerns, and will update all clients. These settings can be further used by the server to save preferences server-side, such as an highlight word list, auto away on/off, and many other things.Obviously, it comes with a new settings page on the client.
This doesn't exactly solve #85 as the focus of this PR wasn't to fix the settings UI, but to provide the backend infrastructure for server settings so we're not stuck waiting for design just to implement features. It just happened that I had to redo the frontend to both to avoid unnecessary pain plugging it back in, as well as showcase the thing in action.
Suggestions welcome. I think it's really pretty good so far, and allows for a lot of future customization.
Demo video: https://d.max-p.me/irc/settings.mp4