Skip to content

Z index feature#2291

Open
desireddymohithreddy0925 wants to merge 4 commits into
Karanjot786:mainfrom
desireddymohithreddy0925:z-index-feature
Open

Z index feature#2291
desireddymohithreddy0925 wants to merge 4 commits into
Karanjot786:mainfrom
desireddymohithreddy0925:z-index-feature

Conversation

@desireddymohithreddy0925

@desireddymohithreddy0925 desireddymohithreddy0925 commented Jul 10, 2026

Copy link
Copy Markdown

Description

This PR enhances the overlay layering system by introducing dynamic z-index management. It adds bringToFront(id) and sendToBack(id) methods to the LayerManager in @termuijs/core, allowing developers to programmatically pop modals or dropdowns to the very top of the rendering stack without hardcoding arbitrary zIndex values.

Related Issue

Closes #2251

Which package(s)?

@termuijs/core

Type of Change

  • 🐛 Bug fix (type:bug)
  • ✨ Feature (type:feature)
  • 📝 Docs (type:docs)
  • 🧪 Tests (type:testing)
  • ♻️ Refactor (type:refactor)
  • 🎨 Design / UX (type:design)
  • ♿ Accessibility (type:accessibility)
  • ⚡ Performance (type:performance)
  • 🔧 DevOps / CI (type:devops)
  • 🔒 Security (type:security)

Checklist

  • ⭐ You starred the repo. The needs-star check blocks your merge otherwise.
  • Tests pass locally: bun vitest run
  • Build passes: bun run build
  • Typecheck passes: bun run typecheck
  • You read CONTRIBUTING.md.
  • Your PR title follows type: short description.
  • Widget state mutators call markDirty() (if your change affects rendering).
  • No new any types without an inline comment explaining why.
  • No unrelated refactors bundled into this PR.

GSSoC 2026 Participation

  • You are a GSSoC 2026 contributor.
  • Your GSSoC profile: https://gssoc.girlscript.org/profile/YOUR_PROFILE_ID_HERE

Screenshots / Recordings (UI changes)

Notes for the Reviewer

These helper methods loop through the currently active layers in LayerManager to find the absolute maximum or minimum z-index currently allocated, ensuring we never cause z-index collisions when dynamically moving layers.

Summary by CodeRabbit

  • New Features
    • Added controls to move a layer above all other layers.
    • Added controls to move a layer below all other layers.
    • Layer ordering remains unchanged when the specified layer does not exist.

@github-actions github-actions Bot added the area:core @termuijs/core label Jul 10, 2026
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

LayerManager now exposes methods to move an identified layer above or below every other layer by recalculating its z-index.

Changes

Layer ordering

Layer / File(s) Summary
Add front and back layer controls
packages/core/src/terminal/LayerManager.ts
Adds bringToFront and sendToBack, which no-op for missing layers and assign z-index values above or below all other layers.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested labels: gssoc:approved, type:feature, level:intermediate

Suggested reviewers: Karanjot786

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning [2251] The PR adds LayerManager helpers, but it does not implement the core zIndex property or renderer sorting required by the issue. Add the core z-index property and update rendering to sort/composite widgets by z-index as described in #2251.
Title check ❓ Inconclusive The title is related, but too vague to clearly describe the LayerManager z-index methods added in this PR. Use a specific type-prefixed title like feature: add LayerManager z-index controls.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description mostly matches the template and includes the issue, package, type, checklist, and reviewer notes.
Out of Scope Changes check ✅ Passed The changes stay focused on z-index layer management in @termuijs/core and do not introduce unrelated code paths.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/core/src/terminal/LayerManager.ts`:
- Around line 118-153: Update bringToFront() and sendToBack() so that whenever
the target layer’s zIndex changes, all visible layers are marked dirty, ensuring
the render loop recomposites immediately after reordering. Reuse the existing
layer visibility and dirty-state APIs in LayerManager rather than changing
z-index behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6a800627-ea4f-474b-bb98-a8d5f2dc6e0b

📥 Commits

Reviewing files that changed from the base of the PR and between 65d1f7e and be2c821.

📒 Files selected for processing (1)
  • packages/core/src/terminal/LayerManager.ts

Comment on lines +118 to +153
/**
* Brings the specified layer to the front by assigning it a z-index higher than all others.
*/
bringToFront(id: string): void {
const layer = this._layers.get(id);
if (!layer) return;

let maxZ = -Infinity;
for (const l of this._layers.values()) {
if (l.id !== id && l.zIndex > maxZ) {
maxZ = l.zIndex;
}
}
if (layer.zIndex <= maxZ) {
layer.zIndex = maxZ + 1;
}
}

/**
* Sends the specified layer to the back by assigning it a z-index lower than all others.
*/
sendToBack(id: string): void {
const layer = this._layers.get(id);
if (!layer) return;

let minZ = Infinity;
for (const l of this._layers.values()) {
if (l.id !== id && l.zIndex < minZ) {
minZ = l.zIndex;
}
}
if (layer.zIndex >= minZ) {
layer.zIndex = minZ - 1;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n## LayerManager outline\n'
ast-grep outline packages/core/src/terminal/LayerManager.ts --view expanded || true

printf '\n## Search for dirty-region and redraw related symbols\n'
rg -n "dirtyRegion|hasDirtyLayers|composite\\(|mark.*Dirty|invalidate|full.*redraw|redraw" packages/core/src/terminal packages/core/src -g '*.ts' || true

printf '\n## Read LayerManager.ts around relevant sections\n'
nl -ba packages/core/src/terminal/LayerManager.ts | sed -n '1,260p'

Repository: Karanjot786/TermUI

Length of output: 10867


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n## LayerManager relevant sections\n'
sed -n '114,320p' packages/core/src/terminal/LayerManager.ts | cat -n

printf '\n## App render loop around layer compositing\n'
sed -n '390,455p' packages/core/src/app/App.ts | cat -n

printf '\n## Renderer invalidate usage\n'
sed -n '90,145p' packages/core/src/terminal/Renderer.ts | cat -n

Repository: Karanjot786/TermUI

Length of output: 14022


Invalidate layers when z-index changes

bringToFront() and sendToBack() only update zIndex. Since the render loop skips compositing when hasDirtyLayers() is false, a layer reorder can be skipped entirely until some other change dirties a layer. Mark the visible layers dirty here so the new stacking order is rendered immediately.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/terminal/LayerManager.ts` around lines 118 - 153, Update
bringToFront() and sendToBack() so that whenever the target layer’s zIndex
changes, all visible layers are marked dirty, ensuring the render loop
recomposites immediately after reordering. Reuse the existing layer visibility
and dirty-state APIs in LayerManager rather than changing z-index behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core @termuijs/core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feature] Z-index and Layering System

1 participant