Add missing tooltips and a reload button

SillyTavern / SillyTavern-Timelines

Timeline view for SillyTavern chats

MIT License

34 stars 4 forks source link

Add missing tooltips and a reload button #16

Closed Technologicat closed 5 months ago

Technologicat commented 5 months ago

As it says on the tin:

Add tooltips to settings panel.
In the timeline view, with a node opened, improve tooltips of the main and branch buttons.
In the timeline view, add a GUI button to reload the graph (same as the slash command /tl r). At least I often find the need to do this, and a button is sometimes more convenient.

Technologicat commented 5 months ago

This small PR is complete, but for the future, the Timelines extension still needs some work.

EDIT: Notes for the future moved to #17.

EDIT: Yes, really complete after the ~two~ three trivial fixes below. This time for sure.

Technologicat commented 5 months ago

@Cohee1207: I have a bunch of stashed changes fixing many of the UX issues I wrote up in #17, including the checkpoint proliferation.

Is it preferable for you if I just push those changes here, or do you want a separate PR?

Cohee1207 commented 5 months ago

You may add it separately

Cohee1207 commented 5 months ago

I'm not sure about the part with the commented-out code. Was it really unused?

city-unit commented 5 months ago

I'm not sure about the part with the commented-out code. Was it really unused?

Yes, it was for the former right click functionality which was overhauled with a bespoke menu rather than the context menu library for cytoscape.

Technologicat commented 5 months ago

I've found some more unused code. I'm proposing to take it out in the next PR, with the UX improvements.

Technologicat commented 5 months ago

@city-unit: While you're here, does the Lock Nodes setting actually do anything?

To me it looks like it doesn't, but since I can't prove that 100%, I feel a bit nervous to take it out.

To my understanding, from the Cytoscape docs, locking prevents nodes from moving while running an animated layout, or manually by dragging. However, we currently always lock the nodes when the layout completes (we only use a static one), whether that setting is enabled or not.

I propose to remove the setting in the next PR.

city-unit commented 5 months ago

@city-unit: While you're here, does the Lock Nodes setting actually do anything?

To me it looks like it doesn't, but since I can't prove that 100%, I feel a bit nervous to take it out.

To my understanding, from the Cytoscape docs, locking prevents nodes from moving while running an animated layout, or manually by dragging. However, we currently always lock the nodes when the layout completes (we only use a static one), whether that setting is enabled or not.

I propose to remove the setting in the next PR.

I think I had put it there so that users could lock/unlock at will to move around the nodes if they felt like it. Up to you if you think it's actually useful.

Technologicat commented 5 months ago

I think I had put it there so that users could lock/unlock at will to move around the nodes if they felt like it. Up to you if you think it's actually useful.

Ok. Thank you!

I think it isn't currently useful, because the nodes are always locked after running the layout. Also, whenever the data changes, the graph needs a relayout anyway. Since there's typically lots of data, the preferred strategy to handle this is to make the automatic layout so good that there's no need to move nodes manually. I think it's already pretty good. I haven't felt the need to move any nodes - only the full info panel sometimes gets in the way (and I'll look into that).

Also, great work on the original extension! This is exactly what ST needed for a multiverse capability. All the hard parts are done already, so all I've needed to slightly improve the UX. :)