Skip to content

Revise z index #5514

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

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Revise z index #5514

wants to merge 4 commits into from

Conversation

inikolova
Copy link
Contributor

@inikolova inikolova commented Jul 15, 2025

Closes https://github.com/telerik/kendo-themes-private/issues/366

This PR addresses the following issues as well:

#5427
#5397
#5025

Based on an audit of z-index usage across all themes and components, I would suggest the following organisation into semantic groups with clearly defined and meaningful ranges:
1. Base / background (-1-0)
For elements that should appear behind normal content (pseudo-elements, placeholders, backgrounds);
2. Content / basic (1-99)
For normal content elements and their internal layering - like grid headers, calendar header, form inputs etc.
3. Navigation (100-999)
For elements that should appear above normal content (for example Sticky Appbar);
4. Floating (1000-9999)
For elements that float above the main interface (Notification, Badge, Dock manager unpinned containers, Map attribution, Fixed Appbar);
5. Overlay (10000-11999)
For modal dialogs and overlays, Drawer overlays, Fullscreen modes.
6. Popups (12000 - 19999)
For Tooltip, Popover, Context menu, Chat tooltip.
7. User feedback (20000-29999)
For drag&drop elements elements, loading states, and temporary user feedback.
8 Special Cases (30000+)
For emergency overrides and special cases (!!!use sparingly).

Here are some further suggestions based on the research:
1. Structural Elements (for example k-header) which are direct children of a component should have z-index 1;
2. k-resize-handle -> its z-index has been increased to 10, as it should always appear above its surrounding elements.; this value is now consistent across all components;
3. Interactive states - when component state needs a higher priority, increase its z-index with 1;
4. .k-edit-cell should have the highest priority than all other cells - normal, sticky, intersection of sticky row and cell, therefore is z-index is increased to 5.

@inikolova inikolova self-assigned this Jul 15, 2025
Copy link

github-actions bot commented Jul 15, 2025

Packages Report

core default classic bootstrap material fluent utils
Size 29.15 KB (0.0%) 781.50 KB (-0.0%🔽) 781.51 KB (-0.0%🔽) 793.57 KB (-0.1%🔽) 927.07 KB (-0.0%🔽) 1151.99 KB (-0.0%🔽) 625.19 KB (0.0%)
Gzip Size 4.85 KB (0.0%) 100.96 KB (-0.1%🔽) 101.25 KB (-0.1%🔽) 103.35 KB (-0.1%🔽) 114.42 KB (-0.1%🔽) 136.67 KB (0.1%🔼) 52.16 KB (0.0%)
Compile Time 2 s (0.0%) 3 s (0.0%) 3 s (0.0%) 4 s (0.0%) 3 s (0.0%) 5 s (0.0%) 3 s (0.0%)

@inikolova inikolova modified the milestone: 2025 Q3 (Aug) Jul 16, 2025
@inikolova inikolova marked this pull request as ready for review July 16, 2025 14:36
@inikolova inikolova requested a review from a team as a code owner July 16, 2025 14:36
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change expected?
Or is this test legacy and no longer relevant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test is legacy, therefore I removed it

@zhpenkov zhpenkov requested a review from Copilot July 17, 2025 10:29
Copy link

@Copilot Copilot AI left a 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 reorganizes z-index values across themes and components to follow a semantic grouping (background, content, navigation, floating, overlays, popups, user feedback, special cases). Key changes include lowering the AppBar default z-index, aligning documentation, and adjusting dozens of component layers to match their intended stack order.

  • Lower AppBar z-index from 1000 to 900 in all themes
  • Update documentation tables to reflect the new default
  • Adjust various component z-indexes (splitter handles, popups, media player, grid, Gantt, draggable, dock-manager, color palette)

Reviewed Changes

Copilot reviewed 36 out of 45 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/material/scss/appbar/_variables.scss Set AppBar z-index to 900
packages/material/docs/customization*.md Align docs with new 900 default
packages/fluent/scss/appbar/_layout.scss Hardcoded fixed AppBar z-index changed to 1000
packages/fluent/scss/spreadsheet/_layout.scss Removed .k-spreadsheet-tabstrip z-index block
packages/fluent/scss/splitter/_layout.scss Increased splitter drag-handle z-index to 10
packages/fluent/scss/popup/_layout.scss Raised popup container z-index to 12000
packages/fluent/scss/mediaplayer/_layout.scss Standardized media player layers to z-index: 1
packages/fluent/scss/grid/_layout.scss Bumped grid resizer to 10 and edit-cell to 5
packages/fluent/scss/draggable/_layout.scss Updated draggable feedback to z-index 20000
packages/fluent/scss/dock-manager/_layout.scss Set dock-manager panel to z-index 1 / 1000
packages/fluent/scss/colorpalette/_layout.scss Refined hover/focus stack levels to 3 / 2
Comments suppressed due to low confidence (1)

packages/fluent/scss/spreadsheet/_layout.scss:871

  • The entire .k-spreadsheet-tabstrip block was removed, including layout tweaks beyond z-index. This may break tabstrip styling or behavior. If only z-index needed adjustment, revert the removal and only update that property.
        border-color: inherit;

@@ -97,7 +97,7 @@
.k-appbar-fixed {
width: 100%;
position: fixed;
z-index: 1;
z-index: 1000;
Copy link
Preview

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

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

This hardcoded z-index bypasses the $kendo-appbar-zindex variable and sets the AppBar into the 'floating' group (1000) instead of the intended 'navigation' range (100–999). Consider using the $kendo-appbar-zindex variable (now 900) to keep consistency and maintainability.

Suggested change
z-index: 1000;
z-index: var( --kendo-appbar-zindex, #{$kendo-appbar-zindex} );

Copilot uses AI. Check for mistakes.

@@ -81,7 +81,7 @@
.k-mediaplayer-seekbar {
width: 100%;
position: absolute;
z-index: 3;
z-index: 1;
Copy link
Preview

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

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

By aligning the seekbar's z-index with the titlebar at the same level (1), you lose the previous stacking order (titlebar 2 vs. seekbar 3). This may cause the seekbar handle to render behind the titlebar. Consider preserving a higher z-index for the seekbar if it needs to overlay the titlebar.

Suggested change
z-index: 1;
z-index: 2;

Copilot uses AI. Check for mistakes.

@@ -94,7 +94,7 @@
.k-appbar-fixed {
width: 100%;
position: fixed;
z-index: 1;
z-index: 1000;
Copy link
Preview

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

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

This literal value should use $kendo-appbar-zindex (currently 900) instead of hardcoding 1000, which places it outside the intended 'navigation' range and duplicates a value from the 'floating' group.

Suggested change
z-index: 1000;
z-index: $kendo-appbar-zindex;

Copilot uses AI. Check for mistakes.

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.

3 participants