Skip to content

luci-theme-bootstrap: use system font stack with CSS custom properties #8082

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

luci-theme-bootstrap: use system font stack with CSS custom properties #8082

wants to merge 1 commit into from

Conversation

@314systems
Copy link

@314systems 314systems commented Nov 9, 2025

  • This PR is not from my main or master branch 💩, but a separate branch ✅
  • Each commit has a valid ✒️ Signed-off-by: <[email protected]> row (via git commit --signoff)
  • Each commit and PR title has a valid 📝 <package name>: title first line subject for packages
  • Incremented 🆙 any PKG_VERSION in the Makefile
  • Tested on: (architecture, openwrt version, browser) ✅
  • ( Preferred ) Mention: @ the original code author for feedback
  • ( Preferred ) Screenshot or mp4 of changes:
  • ( Optional ) Closes: e.g. openwrt/luci#issue-number
  • ( Optional ) Depends on: e.g. openwrt/packages#pr-number in sister repo
  • Description: Replace hardcoded font-family declarations with CSS custom properties (--font-sans, --font-serif, --font-mono) that utilize system UI fonts.

@systemcrash
Copy link
Contributor

systemcrash commented Nov 18, 2025

Can you further detail advantages, how this might improve upon what we have today?

feckert
feckert requested changes Nov 18, 2025
View reviewed changes
@314systems
luci-theme-bootstrap: use system font stack with CSS custom properties
0e17c6c 
Replace hardcoded font-family declarations with CSS custom properties
(--font-sans, --font-serif, --font-mono) that utilize system UI fonts.

Signed-off-by: Akihiro Nagai <[email protected]>
@314systems 314systems requested a review from feckert November 18, 2025 08:33
@feckert
Copy link
Member

feckert commented Nov 20, 2025

@314systems The source code should be fine.
But you are now changing the current fonts.
Could you please describe in more detail why this is necessary as requested by @systemcrash

@314systems
Copy link
Author

314systems commented Nov 20, 2025

@systemcrash
This update replaces hard‑coded font declarations with CSS variables that point to each platform’s system font stack. This centralises the font settings and ensures appropriate fonts are chosen for both Latin and non‑Latin scripts across different operating systems.

feckert
feckert reviewed Nov 20, 2025
View reviewed changes
Copy link
Member

@feckert feckert left a comment

Choose a reason for hiding this comment

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

I fin with that. But my point is that you change the current font.

themes/luci-theme-bootstrap/htdocs/luci-static/bootstrap/cascade.css
body {
background-color: var(--background-color-high);
font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;
font-family: var(-font-sans);
Copy link
Member

@feckert feckert Nov 20, 2025

Choose a reason for hiding this comment

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

You are changing the current font:
--font-sans: ui-sans-serif, system-ui, sans-serif;

themes/luci-theme-bootstrap/htdocs/luci-static/bootstrap/cascade.css
code, pre {
padding: 0 3px 2px;
font-family: Monaco, Andale Mono, Courier New, monospace;
font-family: var(--font-mono);
Copy link
Member

@feckert feckert Nov 20, 2025

Choose a reason for hiding this comment

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

You are changing the current font:
--font-mono: ui-monospace, monospace;

themes/luci-theme-bootstrap/htdocs/luci-static/bootstrap/cascade.css
button,
select {
font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;
font-family: var(--font-sans);
Copy link
Member

@feckert feckert Nov 20, 2025

Choose a reason for hiding this comment

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

You are changing the current font:
--font-sans: ui-sans-serif, system-ui, sans-serif;

themes/luci-theme-bootstrap/htdocs/luci-static/bootstrap/cascade.css

.btn .close, .alert-message .close {
font-family: Arial, sans-serif;
font-family: var(--font-sans);
Copy link
Member

@feckert feckert Nov 20, 2025

Choose a reason for hiding this comment

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

You are changing the current font:
--font-sans: ui-sans-serif, system-ui, sans-serif;

themes/luci-theme-bootstrap/htdocs/luci-static/bootstrap/cascade.css
color: var(--text-color-highest);
margin-bottom: 18px;
font-family: monospace;
font-family: var(--font-mono);
Copy link
Member

@feckert feckert Nov 20, 2025

Choose a reason for hiding this comment

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

You are changing the current font:
--font-mono: ui-monospace, monospace;

themes/luci-theme-bootstrap/htdocs/luci-static/bootstrap/cascade.css
overflow: auto;
vertical-align: top;
font-family: monospace;
font-family: var(--font-mono);
Copy link
Member

@feckert feckert Nov 20, 2025

Choose a reason for hiding this comment

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

You are changing the current font:
--font-mono: ui-monospace, monospace;

themes/luci-theme-bootstrap/htdocs/luci-static/bootstrap/cascade.css
.uci-dialog var {
text-decoration: none;
font-family: monospace;
font-family: var(--font-mono);
Copy link
Member

@feckert feckert Nov 20, 2025

Choose a reason for hiding this comment

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

You are changing the current font:
--font-mono: ui-monospace, monospace;

themes/luci-theme-bootstrap/htdocs/luci-static/bootstrap/cascade.css
color-scheme: light;

--font-sans: ui-sans-serif, system-ui, sans-serif;
--font-serif: ui-serif, serif;
Copy link
Member

@feckert feckert Nov 20, 2025

Choose a reason for hiding this comment

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

And this is not used?

@feckert feckert closed this Nov 20, 2025
@feckert feckert reopened this Nov 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@feckert feckert feckert requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@314systems @systemcrash @feckert