Chore: Only support small / normal / large font sizes by Squidly271 · Pull Request #2416 · unraid/webgui

Squidly271 · 2025-10-03T05:42:49Z

Summary by CodeRabbit

Style
- Removed extreme “Very large” and “Huge” options from Display and Terminal font selectors for a cleaner, more consistent UI.
Bug Fixes
- Normalizes legacy font-size values to supported percentages to prevent display and terminal sizing inconsistencies.
- Adjusts terminal font mappings for improved readability.
Chores
- Automatically migrates older Display/Terminal preferences during upgrade.
- Updated copyright year.
UI Layout
- Increased VM page content spacing to improve layout and scrolling behavior.

coderabbitai · 2025-10-03T05:42:56Z

Walkthrough

Removed two large font options from DisplaySettings, normalized legacy font cookie values in DefaultPageLayout, added an upgrade-time sed migration for font and tty values, increased manualSpacingOffset in VMMachines, and updated copyright years.

Changes

Cohort / File(s)	Summary of Changes
Display settings options trim `emhttp/plugins/dynamix/DisplaySettings.page`	Removed display font options with values `75` and `80` and terminal font options `19` and `21` from selectable lists.
Runtime font normalization `emhttp/plugins/dynamix/include/DefaultPageLayout.php`	Added a normalization mapping for legacy font cookie values: `50` → `56.25`, `75`/`80` → `68.75`; updated header copyright year to 2025.
Upgrade-time migration `sbin/upgrade`	Added sed-based migration (if `.../dynamix.cfg` exists): `80
Layout spacing adjustment `emhttp/plugins/dynamix.vm.manager/VMMachines.page`	Increased `manualSpacingOffset` passed to `fillAvailableHeight` from `30` to `50`.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant User
  participant Browser
  participant DisplaySettings as DisplaySettings.page
  participant Layout as DefaultPageLayout.php
  participant VMPage as VMMachines.page
  participant Upgrade as sbin/upgrade
  participant Config as /boot/config/plugins/dynamix/dynamix.cfg

  rect rgb(238,245,252)
  note right of Upgrade: Upgrade-time migration (if config exists)
  User->>Upgrade: run system upgrade
  Upgrade->>Config: detect & open config
  Upgrade->>Config: sed replace font (50→56.25, 75/80→68.75)
  Upgrade->>Config: sed replace tty (11→13, 19/21→17)
  end

  rect rgb(245,252,238)
  note right of Browser: Runtime rendering & UI
  User->>DisplaySettings: open Display Settings
  DisplaySettings-->>User: show trimmed font option lists
  User->>Browser: sends saved font cookie
  Browser->>Layout: request page render with cookie
  Layout->>Layout: normalize legacy font values (50→56.25, 75/80→68.75)
  Layout-->>Browser: render page with normalized font
  Browser->>VMPage: layout fillAvailableHeight (manualSpacingOffset=50)
  VMPage-->>Browser: adjusted spacing applied
  end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I nibbled through options, trimmed the tall and wide,
Mapped old crumbs to measures so the fonts abide.
During upgrades I hop, fixing tty and size,
Spacing stretched a little so content fits nice.
A rabbit's small tweak—swift, neat, and spry. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit's high-level summary is enabled.
Title Check	✅ Passed	The title succinctly conveys that the pull request restricts font size options to only small, normal, and large, matching the main changes which remove the extreme font sizes from both display and terminal settings.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0f1b45f and ec0ecf3.

📒 Files selected for processing (1)

emhttp/plugins/dynamix.vm.manager/VMMachines.page (1 hunks)

✅ Files skipped from review due to trivial changes (1)

emhttp/plugins/dynamix.vm.manager/VMMachines.page

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

github-actions · 2025-10-03T05:43:23Z

🔧 PR Test Plugin Available

A test plugin has been generated for this PR that includes the modified files.

Version: 2025.10.03.0608
Build: View Workflow Run

📥 Installation Instructions:

Install via Unraid Web UI:

Go to Plugins → Install Plugin
Copy and paste this URL:

https://preview.dl.unraid.net/pr-plugins/pr-2416/webgui-pr-2416.plg

Click Install

Alternative: Direct Download

⚠️ Important Notes:

Testing only: This plugin is for testing PR changes
Backup included: Original files are automatically backed up
Easy removal: Files are restored when plugin is removed
Conflicts: Remove this plugin before installing production updates

📝 Modified Files:

Click to expand file list

emhttp/plugins/dynamix.vm.manager/VMMachines.page
emhttp/plugins/dynamix/DisplaySettings.page
emhttp/plugins/dynamix/include/DefaultPageLayout.php

🔄 To Remove:

Navigate to Plugins → Installed Plugins and remove webgui-pr-2416, or run:

plugin remove webgui-pr-2416

_{🤖 This comment is automatically generated and will be updated with each new push to this PR.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

sbin/upgrade (1)
34-35: Configuration migration logic is correct.

The sed command properly migrates legacy font and TTY size values in the config file:

Font: (80|75) → 68.75, 50 → 56.25

TTY: 11 → 13, (19|21) → 17

The mappings are consistent with the normalization logic in DefaultPageLayout.php (lines 22-33) and align with the PR objectives. The file existence check and extended regex usage are appropriate.

Consider using sed -i.bak instead of sed -i to create a backup before modifying the config file, providing a rollback option if the upgrade encounters issues:
-[ -f "/boot/config/plugins/dynamix/dynamix.cfg" ] && sed -ri -e 's/font="(80|75)"/font="68.75"/g' -e 's/font="50"/font="56.25"/g' -e 's/tty="11"/tty="13"/g' -e 's/tty="(19|21)"/tty="17"/g' "/boot/config/plugins/dynamix/dynamix.cfg"
+[ -f "/boot/config/plugins/dynamix/dynamix.cfg" ] && sed -ri.bak -e 's/font="(80|75)"/font="68.75"/g' -e 's/font="50"/font="56.25"/g' -e 's/tty="11"/tty="13"/g' -e 's/tty="(19|21)"/tty="17"/g' "/boot/config/plugins/dynamix/dynamix.cfg"

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 95bc896 and 0f1b45f.

📒 Files selected for processing (3)

emhttp/plugins/dynamix/DisplaySettings.page (0 hunks)
emhttp/plugins/dynamix/include/DefaultPageLayout.php (2 hunks)
sbin/upgrade (2 hunks)

💤 Files with no reviewable changes (1)

emhttp/plugins/dynamix/DisplaySettings.page

🔇 Additional comments (3)

emhttp/plugins/dynamix/include/DefaultPageLayout.php (2)

2-3: LGTM: Copyright year update.

The copyright years have been correctly updated to 2025.

22-33: LGTM: Font size normalization logic is correct.

The switch statement properly normalizes legacy font size values to the new supported sizes:

50 → 56.25

75 → 68.75

80 → 68.75

The mappings are consistent with the upgrade script in sbin/upgrade and align with the PR objectives to remove extreme font sizes. PHP's loose comparison handles the string-to-number coercion correctly. Unsupported values pass through unchanged, which is the intended behavior.

sbin/upgrade (1)

2-2: LGTM: Copyright year update.

The copyright year has been correctly updated to 2025.

unraid#2411 accounted for Huge / Very Large. Just need to account for Large

Chore: Only support small / normal / large font sizes

0f1b45f

coderabbitai Bot reviewed Oct 3, 2025

View reviewed changes

Chore: Adjust Button placement on VMs page

ec0ecf3

unraid#2411 accounted for Huge / Very Large. Just need to account for Large

ljm42 added the 7.2 label Oct 13, 2025

limetech merged commit 75da9e0 into unraid:master Oct 15, 2025
2 checks passed

Squidly271 deleted the Chore/RemoveExtremeFontSizes branch November 5, 2025 01:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chore: Only support small / normal / large font sizes#2416

Chore: Only support small / normal / large font sizes#2416
limetech merged 2 commits into
unraid:masterfrom
Squidly271:Chore/RemoveExtremeFontSizes

Squidly271 commented Oct 3, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 3, 2025 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 3, 2025 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Squidly271 commented Oct 3, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

github-actions Bot commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔧 PR Test Plugin Available

📥 Installation Instructions:

⚠️ Important Notes:

📝 Modified Files:

🔄 To Remove:

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Squidly271 commented Oct 3, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 3, 2025 •

edited

Loading

github-actions Bot commented Oct 3, 2025 •

edited

Loading