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

[BUG] slide drawer navigation item 'Discover' doesn't have active background when selected #5226

Closed
Abilashinamdar opened this issue Oct 5, 2023 · 5 comments · Fixed by #5432
Assignees
Labels
bug Something isn't working data explorer Issues related to the Data Explorer project discover for discover reinvent good first issue Good for newcomers v2.12.0

Comments

@Abilashinamdar
Copy link
Contributor

Describe the bug

When we open the drawer and select Discover it is not highlighted with an active background color like other items.

To Reproduce
Steps to reproduce the behavior:

  1. Click the hamburger icon at the top left.
  2. Click on the Discover item in the OpenSearch Dashboards collapsible section.
  3. The Discover page will be rendered now.
  4. Again repeat step 1.
  5. See the Discover item doesn't have an active background color.
  6. Other items will have active color if we click and go.

Expected behavior
It should be highlighted with an active background color like other items.

Dashboards Version
v 2.10.0

Screenshots
Screenshot from 2023-10-05 17-22-54

Screenshot from 2023-10-05 17-22-37

Screenshot from 2023-10-05 17-22-02

@Abilashinamdar Abilashinamdar added bug Something isn't working untriaged labels Oct 5, 2023
@ananzh ananzh added discover for discover reinvent data explorer Issues related to the Data Explorer project labels Oct 10, 2023
@ananzh
Copy link
Member

ananzh commented Oct 10, 2023

Nice catch. Thank you!

@ananzh ananzh added good first issue Good for newcomers v2.12.0 and removed untriaged labels Oct 10, 2023
@willie-hung
Copy link
Contributor

Hi @ananzh, I would like to work on this issue. Thanks!

@willie-hung
Copy link
Contributor

willie-hung commented Oct 19, 2023

Overview

While investigating this problem, I discovered that two distinct plugins, discover and data-explorer, are triggered when tapping on the discover tab, leading to the isActive render issue.

Problem

  • In the code, it appears that discover and data-explorer are two different instances of core.application. What is the correlation between them?
  • The bug occurs when the appId is changed while navigating through the sidebar. Please refer to reference 1 for more details. What is the best practice in this scenario? My first thought is to check whether the appId is discover or data-explorer when the button is tapped. Both conditions should render the button with isActive === true.

Screenshot

Screenshot 2023-10-18 at 11 16 29 PM

Reference

1 - The code that leads to two appId

https://github.com/opensearch-project/OpenSearch-Dashboards/blob/336dce65664cd1f541db33325e5787722443d9c6/src/plugins/discover/public/plugin.ts#L276C1-L281C10

2 - The logic that check whether the button is active or not

https://github.com/opensearch-project/OpenSearch-Dashboards/blob/336dce65664cd1f541db33325e5787722443d9c6/src/core/public/chrome/ui/header/nav_link.tsx#L85C1-L85C28

@joshuarrrr @ashwin-pc @ananzh @BSFishy any guidance would be really appreciate!

@wbeckler
Copy link

wbeckler commented Nov 3, 2023

Can you change the logic so it is more resilient and works for either app id?

@willie-hung
Copy link
Contributor

Hi @wbeckler no problem! I am preparing my midterm exam at the moment, but I plan to submit a pull request within the next week or two.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working data explorer Issues related to the Data Explorer project discover for discover reinvent good first issue Good for newcomers v2.12.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants