Skip to content

ENG-1749: Fix Obsidian community plugin automated review findings#1103

Merged
trangdoan982 merged 11 commits into
mainfrom
eng-1749-address-the-risks-identified-in-the-latest-release-to-get
Jun 12, 2026
Merged

ENG-1749: Fix Obsidian community plugin automated review findings#1103
trangdoan982 merged 11 commits into
mainfrom
eng-1749-address-the-risks-identified-in-the-latest-release-to-get

Conversation

@trangdoan982

@trangdoan982 trangdoan982 commented Jun 4, 2026

Copy link
Copy Markdown
Member

https://www.loom.com/share/50b74ac8385a495d9fd4ca27c5126954

Passed the check on a mirroring branch: https://community.obsidian.md/account/plugins/discourse-graphs

Summary

Addresses Obsidian automated review failures for the Discourse Graph Obsidian plugin (v1.3.0 review): ESLint directive hygiene, plugin lifecycle/CSS rules, inline style assignments, and Obsidian DOM APIs (activeDocument, createSpan/createDiv/createEl).

Changes are split across five commits aligned with the store-review fix buckets (directives → lifecycle/CSS → inline styles → DOM helpers → follow-up guideline fixes).

Commits

I structure the PR so that you can view groups of errors that were addressed based on each PR. there are follow-up patches afterwards, but mostly each commit handles one bucket of errors.

Bucket Focus Lint errors Smoke Commit
eslint-directives Normalize eslint-disable / eslint-enable comments 0 n/a cb9f6279 address eslint-directive errors
lifecycle-css Frontmatter hiding via styles.css + body class; remove detachLeavesOfType on unload 0 2/2 pass eb9733bd address bucket 2: add style and detach leaf
inline-styles Tag colors, wikilink handle, canvas text measurement (CSS classes + setCssProps) 0 3/3 pass 100dfdc0 fix(obsidian): replace inline styles with CSS classes and setCssProps
dom-helpers Popout-safe activeDocument; Obsidian createSpan / createDiv / createEl 0 2/2 pass fd321266 fix(obsidian): use Obsidian DOM helpers and activeDocument
(follow-up) Settings logos as React SVG; regex lint; misc guideline fixes 07d2db1b

Smoke tests

Plugin lifecycle and CSS architecture

  • Frontmatter key hiding — Open settings → toggle "Show IDs in frontmatter" off and on while viewing a discourse node (pass: Frontmatterkeys hide/show correctly; no console errors)
  • Context panel layout persistence — Simply removed detachLeaf

https://www.loom.com/share/35f64880b62c480eb3638a6f48b6491b

Inline styles → CSS classes / setCssProps

  • Tag colors and cursor — View discourse tags in the editor; hover and click a tag (pass: Tag colors and pointer cursor look unchanged)
  • Wikilink drag handle opacity — On a canvas, hover and drag a wikilink embed handle (pass: Drag handle visibility/opacity behaves as before)
  • Canvas node text measurement — Create a discourse node on canvas with a long title (pass: Node width/sizing matches previous behavior)

https://www.loom.com/share/8d75238f08454e668f0192679db3e3e2

Obsidian DOM helpers and activeDocument

  • Inline node type picker — Select text in editor and trigger inline node type picker (Cmd/Ctrl+Shift+N flow) (pass: Picker renders and selects a node type correctly)
  • Create-node notice link — Create a discourse node and click the link in the success notice (pass: Notice link opens the new node file)

https://www.loom.com/share/b7c844ff2f644840b90bfd72b7896713

Note

Obsidian store-review tooling (eslint-plugin-obsidianmd, lint:obsidian, store-review CLI) exists locally on the worktree but is NOT included in this PR. It's in a follow-up ticket ENG-1832: Add Obsidian eslint to the project

Made with Cursor


Open in Devin Review

@linear-code

linear-code Bot commented Jun 4, 2026

Copy link
Copy Markdown

ENG-1749

@graphite-app

graphite-app Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

PR size/scope check

This PR is over our review-size guideline.

  • Recommended: ~200 lines changed
  • Acceptable limit: up to 400 lines when well-scoped/self-contained
  • Preferred file count: fewer than 5 files

Please split this into smaller PRs unless there is a clear reason the changes need to land together.

If keeping it as one PR, please add a brief justification covering:

  • What single problem this PR solves
  • Why the files/changes are coupled

@supabase

supabase Bot commented Jun 4, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 6 additional findings in Devin Review.

Open in Devin Review

Comment thread apps/obsidian/styles.css Outdated
…tion[bot]@users.noreply.github.com>

format file to pass ci check
@trangdoan982 trangdoan982 force-pushed the eng-1749-address-the-risks-identified-in-the-latest-release-to-get branch from e0c340a to f439a97 Compare June 4, 2026 22:01
@trangdoan982 trangdoan982 requested a review from mdroidian June 4, 2026 23:27
@mdroidian

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 652d76ad1e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/obsidian/styles.css
Comment thread apps/obsidian/src/utils/registerCommands.ts Outdated
trangdoan982 and others added 2 commits June 12, 2026 13:50
Hide imported relation type IDs that do not match static CSS prefixes, and declare app.setting in obsidian-unofficial.d.ts.

Co-authored-by: Cursor <cursoragent@cursor.com>
@vercel

vercel Bot commented Jun 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
discourse-graph Skipped Skipped Jun 12, 2026 5:56pm

Request Review

Relations now live in relations.json, so static CSS prefix selectors are sufficient.

Co-authored-by: Cursor <cursoragent@cursor.com>
@trangdoan982 trangdoan982 merged commit 7195930 into main Jun 12, 2026
8 checks passed
@trangdoan982 trangdoan982 deleted the eng-1749-address-the-risks-identified-in-the-latest-release-to-get branch June 12, 2026 17:57
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.

2 participants