Skip to content

refactor(ui): replace CSS :has() modal sizing with explicit modal classes - #896

Merged
allenhutchison merged 1 commit into
masterfrom
chore/css-has-to-classes
May 26, 2026
Merged

refactor(ui): replace CSS :has() modal sizing with explicit modal classes#896
allenhutchison merged 1 commit into
masterfrom
chore/css-has-to-classes

Conversation

@allenhutchison

@allenhutchison allenhutchison commented May 26, 2026

Copy link
Copy Markdown
Owner

Summary

The Obsidian plugin registry CSS-lint flagged 5 uses of :has() for modal sizing (registry feedback: "Avoid :has — it can cause significant performance issues due to broad selector invalidation"). Each call site was a .modal.modal-container:has(.X) selector setting width on the modal based on a child content class.

Replaces the pattern with this.modalEl.addClass('mod-X') in each modal's onOpen() plus a flat .modal.mod-X selector. The existing contentEl.addClass() calls stay — those drive other in-modal rules that target the content element.

Also drops the gemini-tool-confirmation-modal rule block, which was dead CSS (no element ever received that class — grep confirms zero source references).

Changes

  • src/ui/rag-status-modal.ts, src/ui/rag-progress-modal.ts, src/ui/background-tasks-modal.ts, src/ui/agent-view/project-picker-modal.ts, src/ui/agent-view/session-list-modal.ts — add this.modalEl.addClass('mod-X') alongside existing contentEl.addClass().
  • styles.css — 4 selectors moved from .modal.modal-container:has(.X) to .modal.mod-X; the gemini-tool-confirmation-modal block (2 dead rules) deleted.
  • src/services/generated-help-references.ts — incidental: caught up the auto-generated file with the eval-suite reference that docs: publish eval-suite methodology + results table (#874, #875) #893 added without committing the regen.

Checklist

Required

  • I have read and agree to the Contributing Guidelines
  • I have read and agree to the AI Policy
  • This PR is linked to an approved issue where the approach was discussed with a maintainer — N/A (registry CSS-lint feedback)
  • All CI checks pass (npm test, npm run build, npm run format-check)
  • I have tested this change on Desktop — build + tests pass; visual verification of modal sizes happens on next reload in the test vault
  • I have verified this change does not break Mobile (or includes appropriate platform guards) — pure CSS/class change, no platform-gated paths
  • Documentation has been updated (if applicable) — N/A, no user-visible behavior change
  • I understand that I must address all review comments from CodeRabbit and maintainers, or this PR may be closed

AI-Generated Code

  • This PR includes AI-generated or AI-assisted code
  • AI tool(s) used: Claude Code
  • I have reviewed and understand all AI-generated code in this PR

Summary by CodeRabbit

  • New Features
    • Added Eval Suite documentation reference to the help system.

Review Change Stack

…sses

The Obsidian plugin registry CSS-lint flagged 5 uses of `:has()` for
modal sizing — broad selector invalidation costs that grow with vault
size. Each call site was a `.modal.modal-container:has(.X)` selector
setting `width` on the modal based on a child content class.

Replaces the pattern with `this.modalEl.addClass('mod-X')` in each
modal's `onOpen()` plus a flat `.modal.mod-X` selector. The existing
`contentEl.addClass()` calls stay — those drive other in-modal rules
that target the content element.

Also drops the `gemini-tool-confirmation-modal` rule block, which was
dead CSS (no element ever received that class).

The `generated-help-references.ts` regen is incidental — it caught up
the auto-generated file with the eval-suite reference that #893 added
without committing the regen.
@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 956605be-6e5c-4fcb-a73a-872682e09870

📥 Commits

Reviewing files that changed from the base of the PR and between 2653088 and b3615cc.

📒 Files selected for processing (7)
  • src/services/generated-help-references.ts
  • src/ui/agent-view/project-picker-modal.ts
  • src/ui/agent-view/session-list-modal.ts
  • src/ui/background-tasks-modal.ts
  • src/ui/rag-progress-modal.ts
  • src/ui/rag-status-modal.ts
  • styles.css
📜 Recent review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.ts: Trigger full plugin reload when settings change
Use vault.getMarkdownFiles() instead of vault.adapter.list() for file operations
Use app.fileManager.processFrontMatter() for frontmatter manipulation
Use vault.getAbstractFileByPath() for file operations
Use app.metadataCache for file metadata access
Use app.fileManager.renameFile() for renaming files to preserve metadata
Use camelCase for variables/functions, PascalCase for classes/types, and kebab-case filenames aligned with their feature area
Ensure all properties are correctly typed in TypeScript; handle TypeScript errors properly
Use proper async/await patterns for all asynchronous operations
Use dedicated Logger service (src/utils/logger.ts) that respects debug mode setting instead of global console patching
Access logger via this.plugin.logger in plugin components, context.plugin.logger in tool implementations, and pass logger as parameter to utility functions
Use logger.log() and logger.debug() for debug information that only outputs when debug mode is enabled; automatically filtered based on settings.debugMode
Use logger.error() and logger.warn() for important errors and warnings always visible regardless of debug mode; use for critical failures and data integrity issues
Never use native console.log() or console.debug() directly; always use the Logger service

Files:

  • src/ui/rag-status-modal.ts
  • src/ui/agent-view/project-picker-modal.ts
  • src/ui/rag-progress-modal.ts
  • src/ui/background-tasks-modal.ts
  • src/ui/agent-view/session-list-modal.ts
  • src/services/generated-help-references.ts
src/ui/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Use app.workspace.openLinkText() for clickable file links in views

Files:

  • src/ui/rag-status-modal.ts
  • src/ui/agent-view/project-picker-modal.ts
  • src/ui/rag-progress-modal.ts
  • src/ui/background-tasks-modal.ts
  • src/ui/agent-view/session-list-modal.ts
**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

**/*.ts: Format code with Prettier: 2-space indent, 120-column width, semicolons, single quotes, trailing commas
Lint with ESLint using eslint-plugin-obsidianmd recommended preset to enforce Obsidian-specific best practices
Enforce rules in eslint.config.mjs including memory-leak prevention, cross-window safety, command-ID conventions, and manifest validation via eslint-plugin-obsidianmd

Files:

  • src/ui/rag-status-modal.ts
  • src/ui/agent-view/project-picker-modal.ts
  • src/ui/rag-progress-modal.ts
  • src/ui/background-tasks-modal.ts
  • src/ui/agent-view/session-list-modal.ts
  • src/services/generated-help-references.ts
src/ui/agent-view/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

src/ui/agent-view/**/*.ts: Implement IConfirmationProvider interface in agent view components (AgentView) and pass to AgentLoop via AgentLoopOptions.confirmationProvider
Classify files in attachment pipeline using classifyFile() from src/utils/file-classification.ts
Enforce cumulative 20 MB size limit for attachments across vault drops, external drops, and paste
Use InlineAttachment (renamed from ImageAttachment) to hold base64 data, MIME type, and optional vault path
Inject binary file inlineData as parts alongside functionResponse in conversation history during tool execution

Files:

  • src/ui/agent-view/project-picker-modal.ts
  • src/ui/agent-view/session-list-modal.ts
🧠 Learnings (8)
📚 Learning: 2025-12-29T00:49:29.597Z
Learnt from: allenhutchison
Repo: allenhutchison/obsidian-gemini PR: 277
File: src/services/rag-indexing.ts:459-512
Timestamp: 2025-12-29T00:49:29.597Z
Learning: Guideline: When using the Obsidian API in TypeScript code, import the standalone setTooltip from 'obsidian' and call setTooltip(element, tooltip, options?) instead of using a component method like component.setTooltip(tooltip, options). This applies to any TypeScript file (not just UI components) and helps avoid confusion between the standalone API and component-specific methods. Ensure the import is: import { setTooltip } from 'obsidian'; and use the signature setTooltip(element: HTMLElement, tooltip: string, options?: { placement?: 'top' | 'bottom' | 'left' | 'right' });

Applied to files:

  • src/ui/rag-status-modal.ts
  • src/ui/agent-view/project-picker-modal.ts
  • src/ui/rag-progress-modal.ts
  • src/ui/background-tasks-modal.ts
  • src/ui/agent-view/session-list-modal.ts
  • src/services/generated-help-references.ts
📚 Learning: 2026-04-14T00:36:00.034Z
Learnt from: allenhutchison
Repo: allenhutchison/obsidian-gemini PR: 628
File: src/services/selection-action-service.ts:118-123
Timestamp: 2026-04-14T00:36:00.034Z
Learning: If this project targets an ES2016 (ES7) TypeScript `lib` (so `String.prototype.replaceAll` is unavailable), replace all occurrences of a fixed placeholder in a string (e.g., `{{selection}}`) using `str.split('{{selection}}').join(replacement)` instead of `replace`/`replaceAll`. This avoids `String.prototype.replace`’s `$`-pattern substitution pitfalls (`$$`, `$&`, `$'`, etc.) when the replacement string can contain arbitrary user-provided text.

Applied to files:

  • src/ui/rag-status-modal.ts
  • src/ui/agent-view/project-picker-modal.ts
  • src/ui/rag-progress-modal.ts
  • src/ui/background-tasks-modal.ts
  • src/ui/agent-view/session-list-modal.ts
  • src/services/generated-help-references.ts
📚 Learning: 2026-03-01T22:53:18.726Z
Learnt from: allenhutchison
Repo: allenhutchison/obsidian-gemini PR: 380
File: src/main.ts:856-868
Timestamp: 2026-03-01T22:53:18.726Z
Learning: For Obsidian's SecretStorage API (v1.11.4+), setSecret(id, secret) and getSecret(id) are synchronous. Do not use await with these calls. After calling setSecret(id, secret), immediately call getSecret(id) and verify the value to confirm storage succeeded before proceeding. If getSecret returns null or a different value, handle the failure. This guideline applies to all code paths in the codebase that use this API.

Applied to files:

  • src/ui/rag-status-modal.ts
  • src/ui/agent-view/project-picker-modal.ts
  • src/ui/rag-progress-modal.ts
  • src/ui/background-tasks-modal.ts
  • src/ui/agent-view/session-list-modal.ts
  • src/services/generated-help-references.ts
📚 Learning: 2026-04-14T00:35:57.612Z
Learnt from: allenhutchison
Repo: allenhutchison/obsidian-gemini PR: 628
File: src/prompts/prompt-manager.ts:102-126
Timestamp: 2026-04-14T00:35:57.612Z
Learning: In this repo (allenhutchison/obsidian-gemini), plugin-owned tags are standardized to the `gemini-scribe/*` namespace (e.g., `gemini-scribe/selection-prompt`). The legacy `selection-action` tag is orphaned dead code and was removed (commit 6c6d605); there is no compatibility requirement to keep it. During code review, do not flag missing `selection-action` tag handling—expect only `gemini-scribe/*` tags to be used.

Applied to files:

  • src/ui/rag-status-modal.ts
  • src/ui/agent-view/project-picker-modal.ts
  • src/ui/rag-progress-modal.ts
  • src/ui/background-tasks-modal.ts
  • src/ui/agent-view/session-list-modal.ts
  • src/services/generated-help-references.ts
📚 Learning: 2026-04-20T02:48:16.135Z
Learnt from: allenhutchison
Repo: allenhutchison/obsidian-gemini PR: 666
File: src/tools/image-tools.ts:96-121
Timestamp: 2026-04-20T02:48:16.135Z
Learning: When writing binary files to the Obsidian vault via `vault.createBinary()`, assume it throws an error if the target path already exists (it does not overwrite). Handle this by catching the exception and logging it (as done in `src/services/rag-cache.ts:125-131`). For concurrent/background generation, avoid file reservation/locking; instead guarantee filename uniqueness (e.g., timestamp + random suffix) consistently across image generation (`generate_image`), research caching (`deep_research`), and any scheduled-task runner code paths.

Applied to files:

  • src/ui/rag-status-modal.ts
  • src/ui/agent-view/project-picker-modal.ts
  • src/ui/rag-progress-modal.ts
  • src/ui/background-tasks-modal.ts
  • src/ui/agent-view/session-list-modal.ts
  • src/services/generated-help-references.ts
📚 Learning: 2026-05-04T22:54:56.817Z
Learnt from: allenhutchison
Repo: allenhutchison/obsidian-gemini PR: 756
File: src/services/hook-runner.ts:66-69
Timestamp: 2026-05-04T22:54:56.817Z
Learning: When implementing or reviewing headless agent runners (e.g., HookRunner, ScheduledTaskRunner) and the related model client code in this repo, scope the allowed skills by passing the skill include-list via `ExtendedModelRequest.projectSkills`. Do not look for or use `session.setEnabledSkills` (it does not exist in this codebase). Both `gemini-client.ts` and `ollama-client.ts` consume `projectSkills` as the include-list. If `projectSkills` is an empty array, it must be treated as "inherit all skills"—do not flag this as a missing filter. Any per-hook/per-task skill restriction must be enforced by honoring `projectSkills` in these headless flows.

Applied to files:

  • src/ui/rag-status-modal.ts
  • src/ui/agent-view/project-picker-modal.ts
  • src/ui/rag-progress-modal.ts
  • src/ui/background-tasks-modal.ts
  • src/ui/agent-view/session-list-modal.ts
  • src/services/generated-help-references.ts
📚 Learning: 2026-03-02T01:02:13.449Z
Learnt from: allenhutchison
Repo: allenhutchison/obsidian-gemini PR: 380
File: src/ui/settings.ts:300-304
Timestamp: 2026-03-02T01:02:13.449Z
Learning: In Obsidian v1.11.4+ the SecretComponent is a dropdown from Obsidian's secret registry. Values passed to onChange come directly from the registry and should not contain extraneous whitespace; do not apply whitespace normalization (e.g., .trim()) for inputs coming from this component. When reviewing code, ensure any handling of values from SecretComponent relies on registry-provided values and avoid trimming or normalizing them unless explicitly justified by downstream representations.

Applied to files:

  • src/ui/rag-status-modal.ts
  • src/ui/agent-view/project-picker-modal.ts
  • src/ui/rag-progress-modal.ts
  • src/ui/background-tasks-modal.ts
  • src/ui/agent-view/session-list-modal.ts
📚 Learning: 2026-04-10T00:49:45.270Z
Learnt from: allenhutchison
Repo: allenhutchison/obsidian-gemini PR: 593
File: src/ui/settings-tools.ts:1-19
Timestamp: 2026-04-10T00:49:45.270Z
Learning: In the obsidian-gemini TypeScript codebase, use `InstanceType<typeof ObsidianGemini>` as the established type for the plugin instance parameter in UI/settings modules (e.g., `plugin: InstanceType<typeof ObsidianGemini>`). During code review, do not change this to `plugin: ObsidianGemini` in individual files; any such type-pattern migration should be done once repo-wide in a dedicated cleanup PR rather than piecemeal edits.

Applied to files:

  • src/ui/rag-status-modal.ts
  • src/ui/agent-view/project-picker-modal.ts
  • src/ui/rag-progress-modal.ts
  • src/ui/background-tasks-modal.ts
  • src/ui/agent-view/session-list-modal.ts
🔇 Additional comments (7)
src/services/generated-help-references.ts (1)

29-29: LGTM!

Also applies to: 53-53, 79-79

src/ui/agent-view/project-picker-modal.ts (1)

34-34: LGTM!

src/ui/agent-view/session-list-modal.ts (1)

41-41: LGTM!

src/ui/background-tasks-modal.ts (1)

108-108: LGTM!

src/ui/rag-progress-modal.ts (1)

62-62: LGTM!

src/ui/rag-status-modal.ts (1)

53-53: LGTM!

styles.css (1)

1461-1464: LGTM!

Also applies to: 3608-3611, 3846-3849, 4409-4412


Walkthrough

This PR extends the help references with evals.md and refactors modal styling across five modals from CSS :has() container selectors to dedicated modifier classes applied directly to modal elements, updating selectors in the stylesheet accordingly.

Changes

Evals Reference

Layer / File(s) Summary
Evals reference registration
src/services/generated-help-references.ts
Import for evals reference module, entry in helpResources map, and row in helpReferencesTable labeled "Eval Suite".

Modal Styling Refactor

Layer / File(s) Summary
Modal modifier class implementation
src/ui/agent-view/project-picker-modal.ts, src/ui/agent-view/session-list-modal.ts, src/ui/background-tasks-modal.ts, src/ui/rag-progress-modal.ts, src/ui/rag-status-modal.ts
Five modals now add dedicated modifier classes to their modal elements: mod-gemini-session-modal (session modals), mod-gemini-activity-modal (background tasks), mod-rag-progress-modal, and mod-rag-status-modal.
CSS selector migration
styles.css
CSS selectors for session, RAG status, RAG progress, and activity modals are converted from :has() container rules to direct modifier-class selectors (.modal.mod-<name>), preserving width and max-width constraints. Tool confirmation modal base sizing rules are removed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐰 A rabbit's ode to cleaner styles:
Modal classes spring to life,
No :has() selector strife,
Evals guide the way forward bright,
CSS refactored just right! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main refactoring: replacing CSS :has() selectors with explicit modal classes for better performance.
Description check ✅ Passed The description is well-structured with a clear summary, detailed changes list, and a comprehensive checklist with all required items marked complete.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/css-has-to-classes

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

@allenhutchison
allenhutchison merged commit fa6b5e8 into master May 26, 2026
7 checks passed
@allenhutchison
allenhutchison deleted the chore/css-has-to-classes branch May 26, 2026 01:27
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.

1 participant