-
Notifications
You must be signed in to change notification settings - Fork 70
fix: remove zwsp unicode usage across themes #5640
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.
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
Draft
epetrow
wants to merge
3
commits into
develop
Choose a base branch
from
remove-zwsp
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Packages Report
|
16da677 to
2e8318f
Compare
85b7edf to
69ecace
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes the use of zero-width space (ZWSP) Unicode character (\200b) across theme components and replaces it with proper CSS height calculations to maintain layout consistency.
Key Changes
- Replaced all instances of
content: "\200b"withcontent: "" - Added explicit height calculations using font-size and line-height variables to preserve spacing behavior
- Fixed initialization of
$kendo-input-sizesvariable fromnullto empty map()
Reviewed Changes
Copilot reviewed 15 out of 42 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/scss/components/window/_layout.scss | Replaced ZWSP with explicit height calculation for empty window titles |
| packages/core/scss/components/timeselector/_layout.scss | Removed ZWSP from timeselector pseudo-elements |
| packages/core/scss/components/table/_layout.scss | Added height calculation and box-sizing for table group rows |
| packages/core/scss/components/skeleton/_layout.scss | Replaced ZWSP with inline-block display for skeleton text |
| packages/core/scss/components/radio/_layout.scss | Added height calculation for radio button wrapper |
| packages/core/scss/components/list/_layout.scss | Removed ZWSP from list elements |
| packages/core/scss/components/input/_variables.scss | Changed $kendo-input-sizes default from null to empty map |
| packages/core/scss/components/input/_layout.scss | Removed ZWSP from input value text |
| packages/core/scss/components/icons/_layout.scss | Added height calculation for icon wrapper |
| packages/core/scss/components/gantt/_layout.scss | Removed ZWSP from gantt table cells |
| packages/core/scss/components/dropdowntree/_layout.scss | Added size-specific height calculations for multiselecttree |
| packages/core/scss/components/column-menu/_layout.scss | Added height calculation for checkbox wrapper in column menu |
| packages/core/scss/components/checkbox/_layout.scss | Added height calculation for checkbox wrapper |
| packages/core/scss/components/calendar/_layout.scss | Removed ZWSP from calendar pseudo-elements |
| packages/core/scss/components/action-sheet/_layout.scss | Added height calculations for icon and checkbox wrappers |
e06db0f to
6992b92
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes: #5639
Overview
The usage of unicode characters has intermittently caused issues with strange looking charactes sometimes being displayed. At the moment this can be observed when using the themes from unpkg, where the problem is related to encoding. Since we are minifying the
all.cssfile and relying on SASS's--style=compressedoption to do so the@charsetdeclaration at the top of the file is being removed (which is stardard behaviour). The problem stems from the lack ofcharsetin the content-type header of the files from unpkg, but we do not seem to have control over that.\200bunicode usage patternsThere are two main usage patters for the zero-width space unicode character at the moment:
What has been tried
\200bwith a simple space" "\\200bhowever this would cause the literal characters\200bto be displayed.lhunit instead of calculating the line height ourselves (e.g.var(--kendo-font-size) * var(--kendo-line-height). I have tried the second approach and there are some edge cases that are tricky and in general it does not seem like a good solution since if someone customizes either of these css variables the content will shift (e.g. checkboxes inside ActionSheet).@charsetafter minification - We could have some script or use a tool like PostCSS to add the@charsetthat is being stripped, but that needs to be tested after we ship the css files to unpkg.