search

glue-viz / glue

Linked Data Visualizations Across Multiple Files
http://glueviz.org
Other
721 stars 152 forks source link

Add subtool for changing window title #2407

Closed Carifio24 closed 1 year ago

Carifio24 commented 1 year ago

This PR contains an implementation of a feature request that has been mentioned a few times in the glue meetings, which is the ability to change viewer titles. This is done via a subtool (under the same tool menu as the tool for moving viewer), which opens a dialog that allows users to change the title of the viewer. This also adds a new icon for the tab-moving icon to help distinguish, since there are now multiple subtools in that menu.

At the viewer level, I implemented this by adding a title callback property to the base ViewerState class. This allows viewer titles to be persisted via a session file as well. Since the title in the state is just a string, we should be able to use this in a non-Qt context as well.

jfoster17 commented 1 year ago

Nice! I'm super excited to have this feature in glue and this looks like a reasonable way to put this in. If I had a nitpick it would be that we now have sort of redundant self.LABEL and self.title attributes.

Additionally, the Table Viewer does not inherit from common.qt.data_viewer.DataViewer (though it does inherit state from ViewerState) so this does not allow the user to rename Table Viewers. That's... mostly okay... because a TableViewer title already shows the name of the data set being display and I assume that's normally what a user would want.

The Table Viewer directly sets the window_title property (from BaseQtViewerWidget) -- just in case we need a third way to refer to this!

Carifio24 commented 1 year ago

It's true that self.LABEL and self.state.title are a bit redundant - this PR essentially sets the meaning of LABEL to be "viewer class name/default title". I wanted the viewer title to be in the state for persistence via a session file, which was my main reason for adding that rather than reusing the existing label. I also didn't want to remove it in case there are any downstream applications that use it - this goes back to the larger-scale question about what's part of the public API.

The table viewer actually does inherit from DataViewer, but this implementation hadn't been adding the tool to it (because I forgot that it has inherit_tools = False), and it wouldn't have worked anyways because of the difference in window titling mechanisms that you pointed out.

I've added another commit that I think helps resolve this - basically, the approach is to leave self.state.title as None until the user explicitly sets it (before I was setting it to LABEL if it was empty or None). This lets us know whether any window title comes from default behavior or not, and do things like keep the current table titling behavior (including updating as the data layer changes) until the user sets the name themselves. I think the table viewer is the only built-in viewer that has any dynamic title functionality, but maybe some custom viewers could use this.