Give users the option to add symbols #55
Conversation
As suggested in issue samwho#54
There was a problem hiding this comment.
This is a really good start! I've left a smattering of review comments.
I'm on the fence about editing the default symbols. On the one hand, it's unobtrusive. On the other I feel like it's fairly easy to screw things up in a way that's tricky to correct should you want to.
symbols.js
Outdated
| window.addEventListener("dblclick", (e) => { | ||
| let target = e.target; | ||
| while(target) { | ||
| console.log(target); |
There was a problem hiding this comment.
| console.log(target); |
symbols.js
Outdated
| window.addEventListener("dragstart", handleDragStart); | ||
| window.addEventListener("dragover", handleDragOver); | ||
| window.addEventListener("dragenter", handleDragEnter); | ||
| window.addEventListener("dragleave", handleDragLeave); |
There was a problem hiding this comment.
Can likely remove this, the handler for it is empty.
There was a problem hiding this comment.
dragover needs to be handled, else the elements are not droppable
dragleave can be removed and is in 914415c
other handlers are not empty, they stay.
|
Preventing the deletion/change of the default symbols is extra code. |
| try { | ||
| // Catching invallid json, not an invalid symbol array | ||
| symbols = JSON.parse(window.localStorage.getItem("symbols")); | ||
| } catch(err) { | ||
| console.error(err); | ||
| } | ||
| if (!symbols) { | ||
| symbols = symbols_default; | ||
| } |
There was a problem hiding this comment.
I was thinking about this earlier today: what happens when the default symbols are updated? I think currently the user won't see them, they'll load what's in local storage and completely ignore the defaults.
We probably need some sort of merging. Alternately, we track users' custom symbols separately to the defaults.
There was a problem hiding this comment.
If you want to merge new defaults, with the existing symbols, we need to label either the user symbols or the defaults (which adds complexity) and then disable the editing of the defaults (which adds complexity) then it will be impossible for users to move the frequently used default symbols to the top of the list, resulting in users that will add existing default symbols also to the user symbols. Reducing the benefits of a list of default symbols.
Another method is to track all the changes made by the user (which adds way more complexity) and apply them every time the defaults are loaded (which is so complex I cannot imagine how to do that)
If you can program that, you're welcome to do that. I probably don't have the knowledge, and if I did my time is also limited.
Another solution would be to publish sets of symbols, including a default. When a set (e.g. the default set) is updated let the user load that set into their collection. That's something I could implement in a few hours. As a matter of fact, I already did. It's in pull request #59
|
Sorry I've not gotten around to taking a closer look at this yet. I'll be travelling for work next week as well, so it's likely I won't get to this for a couple of weeks. I really appreciate the time you've put in to this, so I'm sorry for the delay. |
|
O.K. I've scratched even more itches, but I won't bother you with yet another pull request. |
|
@technetium sorry for the delay on this, but I've decided to not merge this PR. It adds a level of complexity I don't think I want to maintain in the tool. You have my blessing to fork and re-host if this is something that makes a substantial difference to your workflow. I appreciate the time and effort you've put in to contributing to symbol.wtf <3 |
As suggested in issue #54
I've added a "+"-button at the right side of the search field. Feel free to style the button as you like.
All symbols can be edited by double clicking it. Double clicking the name will open a editbox for the name.
Clicking the trashcan will remove the symbol. (Trashcan is styled in CSS, feel free to change it)
Suggestion: Change the text: "Open a PR!" to "Add it yourself" with a link to the addSymbol function.
That will save to some pull requests and discussions about what symbols to include.
Then the only discussion will be what will be a good set of default symbols.
Also made the symbols draggable, so users can get them in their prefered order.
When a symbol is dragged the opacity is reduced to 40%,
use the drag class to change the style of dragged symbols.
The target of the symbol gets a solid border,
the class over can be used to change that.