Skip to content

Conversation

sandbergja
Copy link
Contributor

https://issues.apache.org/jira/browse/SOLR-17936

Description

In the new ui, add a focus indicator around the sidebar entry that has focus.

Before (light mode) -- the cluster element has keyboard focus but there is no way to know that:
a light nav bar. environment is selected but all other entries look the same

Before (dark mode):

a dark nav bar. environment is selected but all other entries look the same

After (light mode):
a light nav bar. environment is selected. cluster has a light gray box around it

After (dark mode):
a dark nav bar. environment is selected. cluster has a light gray box around it

Solution

Use Compose's onFocusChanged functionality to determine when an element has focus, and add a border around it if so. Thanks to this guidance for the approach.

Tests

I wasn't sure how to write an effective automated test for this; I couldn't find a way to make assertions about modifiers or color alpha values in compose -- it seemed like screenshot testing is encouraged more for this type of thing? I'd be happy to add a test if there are some ideas about the approach (or if I could write this functionality in a more testable way)!

I tested this manually by:

  1. Running ./gradlew :solr:ui:run, connecting to my local solr, and tabbing through the various sidebar entries
  2. Building solr locally, opening the new UI in the browser, connecting to my local solr, and tabbing through the various sidebar entries
  3. Turning dark mode on in my OS and repeating the above steps.

Checklist

Please review the following and check all that apply:

  • I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
  • I have developed this patch against the main branch.
  • I have run ./gradlew check.
  • I have added tests for my changes.
  • I have added documentation for the Reference Guide

Copy link
Contributor

@malliaridis malliaridis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution, and so quick as well.

I am not sure if this is the behavior we want to apply here, as a border is always applied now when an element is focused or selected (see screenshots). We are currently using the Material 3 library for most of our elements, and these come with multiple states and visualizations of each state, so that we do not have to define them on our own. The root cause as mentioned in the ticket is that we customize the colors of a Tab element, overriding some default state behavior when it comes to highlighting in focus and hover states. I believe the standard Material Tab element is not the best for what we need here "color-wise". I would therefore try to find either a different UI element (other than Tab, maybe ToggleButton) that comes with the wanted coloring behavior for various states, including default, selected, hovered, focused and disabled.

By using backgrounds for state representation would also not overlap with borders applied by screen readers and browsers.

FYI the dark theme navigation elements have a proper highlighting compared to the light theme, so you can have a look what the "almost" expected behavior is.

I can look into defining tests for these scenarios once we have something that fully fixes the wrong colors in the navigation elements of the side bar.

Screenshot 2025-09-30 at 15 14 29 Screenshot 2025-09-30 at 15 13 27

@malliaridis
Copy link
Contributor

Btw. in case you didn't know, you can use ./gradlew tidy for formatting code (if you haven't configured your IDE to do it) and ./gradlew check -x test for running valuable checks without tests (quick checks).

I haven't updated the dev-docs for the IDE configuration in regards to the formatting, but I plan to. So stay tuned. :D

@sandbergja
Copy link
Contributor Author

Closing in favor of #3744

@sandbergja sandbergja closed this Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants