Fix noto sans cyrillic#8015
Conversation
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Since
Noto Sansis now loaded with a full 100–900 weight range, consider whether all those weights are actually used in the dashboard to avoid unnecessary font payload and reduce page load overhead. - You added
websearch_firecrawl_keyas a list default in the config; double-check that the rest of the code handling this key expects a list (not a string or single token) to avoid type mismatches at runtime.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Since `Noto Sans` is now loaded with a full 100–900 weight range, consider whether all those weights are actually used in the dashboard to avoid unnecessary font payload and reduce page load overhead.
- You added `websearch_firecrawl_key` as a list default in the config; double-check that the rest of the code handling this key expects a list (not a string or single token) to avoid type mismatches at runtime.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request introduces a new configuration key for Firecrawl web search and updates the dashboard's font configuration to include 'Noto Sans'. Feedback indicates that the Firecrawl addition is out of scope for this PR and lacks required unit tests. Additionally, there is a recommendation to include 'Noto Sans SC' in the CJK font fallback list to ensure the loaded webfont is correctly utilized when local system fonts are missing.
| "websearch_bocha_key": [], | ||
| "websearch_brave_key": [], | ||
| "websearch_baidu_app_builder_key": "", | ||
| "websearch_firecrawl_key": [], |
There was a problem hiding this comment.
This change appears to be unrelated to the PR's objective of fixing Cyrillic fonts. It adds a configuration key for Firecrawl web search, which should ideally be handled in a separate pull request to maintain a clean and focused commit history. Additionally, per repository guidelines, new functionality should be accompanied by corresponding unit tests.
References
- New functionality, such as handling attachments, should be accompanied by corresponding unit tests.
| $font-size-root: 1rem; | ||
| $border-radius-root: 8px; | ||
| $cjk-sans-fallback: 'PingFang SC', 'Hiragino Sans GB', 'Noto Sans CJK SC', 'Microsoft YaHei' !default; | ||
| $cjk-sans-fallback: 'PingFang SC', 'Hiragino Sans GB', 'Noto Sans CJK SC', 'Microsoft YaHei', 'Noto Sans' !default; |
There was a problem hiding this comment.
The Google Font loaded in index.html is Noto Sans SC, but the fallback list uses Noto Sans CJK SC. While the latter is a common local font name, the webfont will not be used as a fallback unless 'Noto Sans SC' is explicitly included in the list. Consider adding it after the system fonts to ensure the Chinese webfont works as intended when local CJK fonts are missing.
$cjk-sans-fallback: 'PingFang SC', 'Hiragino Sans GB', 'Noto Sans CJK SC', 'Microsoft YaHei', 'Noto Sans SC', 'Noto Sans' !default;5774f53 to
d120beb
Compare
The WebUI only loaded Noto Sans SC (Simplified Chinese), which lacks Cyrillic glyphs. Russian text fell back to system sans-serif, causing poor rendering depending on the OS. Changes: - Load Noto Sans (regular) from Google Fonts alongside Noto Sans SC - Add 'Noto Sans' at the END of $cjk-sans-fallback (after CJK fonts) so Chinese text still renders with system CJK fonts first, while Cyrillic text falls through to Noto Sans. This ensures both Chinese and Cyrillic text render correctly.
d120beb to
4a0c3e3
Compare
The WebUI currently loads
Noto Sans SC(Simplified Chinese variant) from Google Fonts, which does not include Cyrillic glyphs. As a result, Russian text falls back to the systemsans-serif, causing inconsistent and often poor rendering depending on the user's OS.This change ensures Cyrillic text renders correctly while preserving optimal CJK text rendering.
Modifications / 改动点
index.html: Added
family=Noto+Sansto the Google Fonts URL (loaded alongside existingNoto+Sans+SC)_variables.scss: Added
'Noto Sans'at the end of$cjk-sans-fallbackso Chinese text still prefers system CJK fonts first, while Cyrillic text falls through toNoto SansThis is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
Verified locally: Russian text in WebUI now renders with
Noto Sansinstead of system fallback.Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Improve dashboard font support for Cyrillic text and extend default web search configuration keys.
Bug Fixes:
Enhancements: