Skip to content

feat(settings): improve battery status layout - #3103

Open
hthienloc wants to merge 7 commits into
AvengeMedia:masterfrom
hthienloc:feat/battery-status-layout
Open

feat(settings): improve battery status layout#3103
hthienloc wants to merge 7 commits into
AvengeMedia:masterfrom
hthienloc:feat/battery-status-layout

Conversation

@hthienloc

@hthienloc hthienloc commented Aug 18, 2026

Copy link
Copy Markdown
Member

Type of change

  • Refactor / internal cleanup

Screenshots / video

dms_capture_1787184832770 dms_capture_1787184923595

@hthienloc
hthienloc marked this pull request as draft August 18, 2026 02:36
@hthienloc

hthienloc commented Aug 18, 2026

Copy link
Copy Markdown
Member Author

@bbedward @purian23 My initial intention is to redesign the battery info tab in the settings, but there is a question I want to clarify before going any further:

Should we repeat the information that's already available in the battery widget?

Because I'd like to remove the duplicate information and replace it with something more useful, like battery history, so users can track their battery level over an extended period of time. But the edge case is some user might not use the battery widget so this is still holding some use case.

@purian23

Copy link
Copy Markdown
Collaborator

Hey @hthienloc, I'm fairly open on this one with no strong opinions either way. It should repeat maybe the basics of info, but otherwise open to the options you listed for sure. Thanks!

@hthienloc
hthienloc marked this pull request as ready for review August 20, 2026 00:17
@hthienloc

Copy link
Copy Markdown
Member Author

/claude review

spacing: Theme.spacingXXS

StyledText {
text: I18n.tr("Estimated time")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These re-cased/reworded labels create brand-new untranslated terms and orphan already-translated ones. quickshell/translations/en.json has Estimated Time (line 8809), Battery Health (3262) and AC Adapter (Plugged In) (964), each referenced only from this file — so this PR drops every existing locale's translation for them in exchange for identical strings that differ only in casing/wording. CONTRIBUTING asks to reuse existing terms.

Please restore the exact existing terms:

  • line 147: I18n.tr("Estimated Time")
  • line 177: I18n.tr("Battery Health")
  • line 105: I18n.tr("AC Adapter (Plugged In)")
Suggested change
text: I18n.tr("Estimated time")
text: I18n.tr("Estimated Time")

width: parent.width
iconName: "battery_charging_full"
title: I18n.tr("Status")
settingKey: "batteryStatusCard"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Dropping title: here breaks the settings-search label for this card. extract_settings_index.py falls back to the block's first text: when there's no title (find_settings_components, lines 368-377), so it picked up the no-battery fallback string: the regenerated index now lists batteryStatusCard as "Power" (and with no icon) instead of "Status". Users searching settings will see the battery card titled "Power" under the "Power & Security" tab, which reads as a different setting.

If the header-less card is intentional, keep the searchable title but hide the header — or at minimum reorder so the first text: in the block is a meaningful label rather than the !batteryAvailable fallback.

@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown

Claude review

Layout change looks sound; two i18n/search-index issues worth fixing before merge.

  • New term variants orphan existing translations (Estimated time/Battery health/Plugged In (AC) vs already-translated Estimated Time/Battery Health/AC Adapter (Plugged In)), quickshell/Modules/Settings/BatteryTab.qml:147
  • Removing title: makes the settings-search index label the card "Power" (the no-battery fallback string) instead of "Status", quickshell/Modules/Settings/BatteryTab.qml:58

Checked: effective diff is BatteryTab.qml + settings_search_index.json (the BatteryService.qml commits are already on master); verified Theme.withAlpha/surfaceTextMedium/spacingXXS/fontSizeXLarge, DankIcon.implicitWidth == size, BatteryService.isLowBattery/batteryAvailable, no binding loops in the new Row/Column nesting, and Math.max(0, parent.width - 100) stays within bounds at the 150% max font scale. Model: claude-opus-5.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants