Skip to content

Make settings section separators visible in black theme#7039

Open
mvanhorn wants to merge 3 commits into
element-hq:developfrom
mvanhorn:fix/6852-oled-divider-visibility
Open

Make settings section separators visible in black theme#7039
mvanhorn wants to merge 3 commits into
element-hq:developfrom
mvanhorn:fix/6852-oled-divider-visibility

Conversation

@mvanhorn

Copy link
Copy Markdown

Content

Adds an OLED-aware divider color so the ListSectionHeader section separators stay visible in the Black (OLED) theme.

The Black theme is built in ElementTheme.kt as compoundDark.copy(bgCanvasDefault = Color.Black) - it forces the canvas to pure black but leaves the divider color at its standard-dark value. The section divider resolves to Material's DividerDefaults.color (outlineVariant = colorAlphaGray400, ~10% alpha white), which is visible against the standard dark-gray canvas but almost invisible against pure black.

This PR introduces a listSectionDivider alias in ColorAliases.kt that returns a higher-contrast gray (colorAlphaGray700) only when the OLED/Black theme is active (!isLight && bgCanvasDefault == Color.Black), and wires it into the ListSectionHeader divider. The light and standard-dark themes return the exact same colorAlphaGray400 token they use today, so they are unchanged.

Motivation and context

Closes #6852

In the Black (OLED) theme the section separators and ListSectionHeader dividers in Settings, Developer options, and Advanced settings became almost invisible, making the screens read as one undifferentiated list. The fix is scoped to the divider alias used by section separators so only the OLED case changes.

Screenshots / GIFs

UI change is covered by the existing ListSectionHeaderWithDividerPreview Composable preview, so the divider change is visible in the diff and in screenshot tests. The light and standard-dark snapshots are unchanged because their divider color is byte-for-byte identical to before.

Tests

  • Black/OLED theme: ListSectionHeader dividers render with visible contrast against the pure-black canvas.
  • Standard dark theme: divider color is unchanged (returns the same colorAlphaGray400 token as before).
  • Light theme: divider color is unchanged.
  • hasDivider = false section headers still render no divider in all themes.

Tested devices

  • Physical
  • Emulator
  • OS version(s):

Checklist

  • I am aware of the etiquette.
  • This PR was made with the help of AI:
    • Yes. In this case, please request a review by Copilot.
    • No.
  • Changes have been tested on an Android device or Android emulator with API 24
  • UI change has been tested on both light and dark themes
  • Pull request is based on the develop branch
  • Pull request title will be used in the release note, it clearly defines what will change for the user
  • You've made a self review of your PR

In the Black (OLED) theme the canvas is forced to pure black while the
section-separator divider keeps its standard-dark colour (outlineVariant,
~10% alpha white), leaving dividers in Settings and related screens almost
invisible. Add an OLED-aware listSectionDivider alias that lightens the
divider only when the black theme is active, and use it for the
ListSectionHeader divider. Light and standard-dark themes are unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
@mvanhorn mvanhorn requested a review from a team as a code owner June 17, 2026 08:58
@mvanhorn mvanhorn requested review from jmartinesp and removed request for a team June 17, 2026 08:58
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions

Copy link
Copy Markdown
Contributor

Thank you for your contribution! Here are a few things to check in the PR to ensure it's reviewed as quickly as possible:

  • If your pull request adds a feature or modifies the UI, this should have an equivalent pull request in the Element X iOS repo unless it only affects an Android-only behaviour or is behind a disabled feature flag, since we need parity in both clients to consider a feature done. It will also need to be approved by our product and design teams before being merged, so it's usually a good idea to discuss the changes in a Github issue first and then start working on them once the approach has been validated.
  • Your branch should be based on origin/develop, at least when it was created.
  • The title of the PR will be used for release notes, so it needs to describe the change visible to the user.
  • The test pass locally running ./gradlew test.
  • The code quality check suite pass locally running ./gradlew runQualityChecks.
  • If you modified anything related to the UI, including previews, you'll have to run the Record screenshots GH action in your forked repo: that will generate compatible new screenshots. However, given Github Actions limitations, it will prevent the CI from running temporarily, until you upload a new commit after that one. To do so, just pull the latest changes and push an empty commit.

@github-actions github-actions Bot added the Z-Community-PR Issue is solved by a community member's PR label Jun 17, 2026
@jmartinesp

Copy link
Copy Markdown
Member

Makes sense to me, but we need some design feedback: @americanrefugee ?

@mvanhorn could you attach some screenshots so we can check the result?

@bmarty bmarty left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I guess we should use a color from our Element theme instead of DividerDefaults.color in fun HorizontalDivider?

@bmarty

bmarty commented Jun 17, 2026

Copy link
Copy Markdown
Member

I guess we should use a color from our Element theme instead of DividerDefaults.color in fun HorizontalDivider?

I will create a first PR with this

@americanrefugee

Copy link
Copy Markdown

I guess we should use a color from our Element theme instead of DividerDefaults.color in fun HorizontalDivider?

Use color/separator/primary

@bmarty bmarty mentioned this pull request Jun 18, 2026
12 tasks
@bmarty

bmarty commented Jun 18, 2026

Copy link
Copy Markdown
Member

I guess we should use a color from our Element theme instead of DividerDefaults.color in fun HorizontalDivider?

Use color/separator/primary

Actually the Divider height was not correct. With 1 instead of 0.5 and color/separator/secondary, this is fine. I have done this in #7054 , you can see the recorded screenshots there.

@bmarty

bmarty commented Jun 18, 2026

Copy link
Copy Markdown
Member

You can now fix the separator color by setting a color for separatorSecondary here: https://github.com/element-hq/element-x-android/blob/develop/libraries/compound/src/main/kotlin/io/element/android/compound/theme/ElementTheme.kt#L109

Following review feedback: now that the global HorizontalDivider uses
separatorSecondary (element-hq#7054), give the Black/OLED theme a visible
separatorSecondary value in ElementTheme instead of carrying a bespoke
listSectionDivider alias. This fixes all separators on pure black, not
just section headers. The exact color is pending design confirmation.
@mvanhorn

Copy link
Copy Markdown
Author

Thanks @bmarty, done. Now that #7054 makes the global HorizontalDivider use separatorSecondary, I dropped the bespoke listSectionDivider alias and instead set separatorSecondary on the Black theme in ElementTheme (pushed e02ed32). That makes every separator visible on pure black, not just the settings section headers.

I went with DarkColorTokens.colorGray400 as a conservative first pass. @americanrefugee, happy to bump it lighter (e.g. colorGray500) if you'd prefer more contrast against pure black. I'll add before/after screenshots of the Black theme once CI is green.

@codecov

codecov Bot commented Jun 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.58%. Comparing base (377cca9) to head (e02ed32).
⚠️ Report is 73 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #7039      +/-   ##
===========================================
- Coverage    80.58%   80.58%   -0.01%     
===========================================
  Files         2676     2676              
  Lines        76236    76238       +2     
  Branches     10251    10251              
===========================================
  Hits         61436    61436              
- Misses       10870    10872       +2     
  Partials      3930     3930              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jmartinesp

jmartinesp commented Jun 30, 2026

Copy link
Copy Markdown
Member

@mvanhorn it seems like everything's fine except we need new screenshots to be recorded. Could you do that following the instructions in this automated PR comment?


Actually, it looks like you also need to sign the CLA: #7039 (comment)

compoundDark.copy(bgCanvasDefault = Color.Black)
compoundDark.copy(
bgCanvasDefault = Color.Black,
// Pending design confirmation from americanrefugee/bmarty.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please remove this comment, so that we can merge the PR. Better to discuss decision in GH issues or PR than in the code.

Also please fix the other points (CLA and screenshots).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Z-Community-PR Issue is solved by a community member's PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Section separators in settings almost invisible in black OLED mode

5 participants