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

[MD] Create data source selectable menu using context menu and allow it to be consumed by plugins via Nav bar #6035

Closed
2 tasks done
Tracked by #5872
BionIT opened this issue Mar 5, 2024 · 7 comments
Assignees
Labels
enhancement New feature or request multiple datasource multiple datasource project v2.14.0

Comments

@BionIT
Copy link
Collaborator

BionIT commented Mar 5, 2024

Is your feature request related to a problem? Please describe.
In #5717, we implemented a data source picker in order to unify the experience when selecting data source in devtools, tutorial, etc. Based on requirement to place a data source selector into the navigation bar, we need to implement a new component based on context menu and be able to be consumed by external plugins via TopNavMenu.

Describe the solution you'd like
Implement a new component using context menu and allow external plugins to consume via TopNavMenu. The use of picker will be controlled by showDataSourceMenu property which is a boolean. We should also expose dataSourceMenuConfig which will contain customize options, eg, default option, disabled, filterFn, etc

There will be 2 states(read only vs selectable) for the data source selector in the nav bar, plugins need to decide which state to use based on which page user is at

Describe alternatives you've considered
Instead of having the selector in the nav bar, we can just let consumers to use the regular combo box based picker in the page.

Additional context

Add any other context or screenshots about the feature request here.

@BionIT BionIT added enhancement New feature or request multiple datasource multiple datasource project labels Mar 5, 2024
@BionIT BionIT self-assigned this Mar 5, 2024
@BionIT BionIT removed the untriaged label Mar 5, 2024
@RamakrishnaChilaka
Copy link

Thanks Lu.

There will be 2 states(read only vs selectable) for the data source selector in the nav bar, plugins need to decide which state to use based on which page user is at

Can you please consider having this as an additional prop to know which component to mount, i.e., selectable vs readonly ?

@BionIT
Copy link
Collaborator Author

BionIT commented Mar 7, 2024

Thanks Lu.

There will be 2 states(read only vs selectable) for the data source selector in the nav bar, plugins need to decide which state to use based on which page user is at

Can you please consider having this as an additional prop to know which component to mount, i.e., selectable vs readonly ?

Hi @RamakrishnaChilaka, for readonly, we will need to have a separate component for plugins to use since the behavior would be different for readonly and selectable based on current requirement. We can go through how it would look like in the plugin you work on and see if we can make it easy to consume on your side

@seraphjiang
Copy link
Member

Hi @RamakrishnaChilaka, for readonly, we will need to have a separate component for plugins to use since the behavior would be different for readonly and selectable based on current requirement. We can go through how it would look like in the plugin you work on and see if we can make it easy to consume on your side

+1 for not mixing the two components into one by following single responsibility principle.

actually, datasource context component might be more suitable name for second component base on requirement we know now. Though name is a little bit longer than readonly

@BionIT BionIT changed the title [MD] Create data source menu using context menu and allow it to be consumed by plugins via Nav bar [MD] Create data source selectable menu using context menu and allow it to be consumed by plugins via Nav bar Mar 7, 2024
@RamakrishnaChilaka
Copy link

Got it, Thanks Lu, Huan. Summarising offline discussion with Lu regarding passing dataSourceLabel as well to dataSourceCallBackFunc.

We need this because, Taking an example use case:
For ISM plugin, we need to show data source label in the index detail page and In this page, data source picker is in readonly mode. Hence, the label needs to be passed from the parent component, so it needs to be maintained in the parent state as well.

@derek-ho
Copy link
Contributor

derek-ho commented Mar 7, 2024

+1. Also adding context from security plugin perspective. Since we are going with a top nav menu header registration model, each individual tab will create a new instance of the ClusterSelector/DataSource Context component. Thus, we need some way to manage state within our plugin and be able to render the picker with a specific datasource selected. Thus, what ever datasource one is looking at for one tab will persist as they click to different tabs. Another use case where this may be helpful - some actions such as creation tabs where we are not allowing users to change cluster - I think is impossible to do without being able to pass in cluster selector (since it would default to local cluster with the picker disabled today).

@RamakrishnaChilaka
Copy link

RamakrishnaChilaka commented Mar 21, 2024

@BionIT, Currently, DataSourceMenu takes both dataSourceId & dataSourceLabel for the selectedOption Prop in read only mode. This was required before because we expected plugins to render dataSourceLabel in a P/Div/custom label tag in read-only mode. However, the current data source menu implementation (in the main branch) handles the rendering for read-only mode. So, I think plugins don't need to store dataSourceLabel in their state. Only dataSourceId should be enough to determine which option to show as a label in read-only mode, correct?

@derek-ho, I believe, as data source id is unique across data sources, it can be used to identify which option to show as label in read-only mode.

@BionIT
Copy link
Collaborator Author

BionIT commented Mar 27, 2024

@derek-ho helped to create another issue to allow getting data source label when only data source id gets passed in #6274, and this existing issue can be closed since the selectable component have been exposed and can be mounted using the data source menu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request multiple datasource multiple datasource project v2.14.0
Projects
None yet
Development

No branches or pull requests

4 participants