Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose current tab key in Tabs widget #1390

Open
bheisler opened this issue Nov 11, 2020 · 2 comments
Open

Expose current tab key in Tabs widget #1390

bheisler opened this issue Nov 11, 2020 · 2 comments
Labels
enhancement adds or requests a new feature widget concerns a particular widget

Comments

@bheisler
Copy link
Contributor

Currently, the Tabs widget does not expose the currently-selected tab to outside code. This is a problem for me, as I would like to be able to set the currently-selected tab programmatically (example: to open a new tab and switch focus to it immediately), and react to the user changing it (ex. to update the window menu with commands relevant to the current tab). What I'm unsure of is the best way to implement this change.

One option would be to store the current tab key in the application's data model with a Lens; this is the usual way to do things like this in Druid, but here it's a bit more complex because the Tabs widget cannot implement Widget<TabKey>; because it contains the body widgets it must implement Widget<ApplicationData>. That means that the Tabs widget couldn't use the normal LensWrap type, but instead would need to allow the application developer to configure a specific lens for the tab key - an unfortunate special-case in an otherwise uniform design.

Another option might be to expose a number of Command selectors that the Tabs widget could submit or listen for to notify/be notified about changes to the selected tab. This would also go against the grain of the existing design, and has the added downside that the tab key - the payload for these commands - would have to be type-erased, as the key type is determined by the tab policy. It would also be challenging for application developers to have multiple nested Tabs widgets behave correctly, where the normal reactive model makes that trivial.

A third option would be to extend the existing TabPolicy trait in some way. It could provide a lens, or act as a lens itself. It already does this to some extent, so this would fit in with the existing designs better than the other options.

@cmyr cmyr added enhancement adds or requests a new feature widget concerns a particular widget labels Nov 13, 2020
@cmyr
Copy link
Member

cmyr commented Nov 13, 2020

@rjwittams would be best positioned to evaluate what makes sense here, I really haven't used tabs much yet.

My gut instinct would be to find the solution that a) works and b) is minimally invasive; I suspect this might be to augment the TabPolicy, or even just find away to get this via some method on the Tabs widget itself?

@rjwittams
Copy link
Collaborator

The right way to do this imo would be bindings. The tab index is private state to the widget that you want to optionally expose into your reactive state.
But bindings is probably never going to happen because Crochet will take over.

rjwittams added a commit to rjwittams/druid that referenced this issue Feb 20, 2021
This is mainly for composite widgets that contain a scope: using this they can affect the widgets inside the scope.
Using this facilty, allow getting and setting the tab index of a Tabs widget.
This offers part of a solution to linebender#1390, although it is in terms of the index and not the key.
maan2003 pushed a commit to maan2003/druid that referenced this issue Dec 7, 2021
This is mainly for composite widgets that contain a scope: using this they can affect the widgets inside the scope.
Using this facilty, allow getting and setting the tab index of a Tabs widget.
This offers part of a solution to linebender#1390, although it is in terms of the index and not the key.
maan2003 added a commit that referenced this issue Dec 30, 2021
* Expose scope state.

This is mainly for composite widgets that contain a scope: using this they can affect the widgets inside the scope.
Using this facilty, allow getting and setting the tab index of a Tabs widget.
This offers part of a solution to #1390, although it is in terms of the index and not the key.

* Add suggestions from review

Co-authored-by: Robert Wittams <robert@wittams.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement adds or requests a new feature widget concerns a particular widget
Projects
None yet
Development

No branches or pull requests

3 participants