feat: allow unlabeled SmartBlock buttons#135
Conversation
WalkthroughAdd a new exported parser Changes
Sequence Diagram(s)sequenceDiagram
participant DOM as DOM/Button
participant Observer as Button Observer
participant Parser as parseSmartBlockButton
participant Core as SmartBlock Core
participant Workflows as Workflow Registry
Observer->>DOM: observe element
Observer->>Parser: parseSmartBlockButton(label, text)
Parser-->>Observer: {index, full, buttonContent, buttonText, workflowName, variables}
Observer->>DOM: mark element & attach click handler
DOM->>Observer: Click
Observer->>Workflows: resolve workflowName -> srcUid
alt Workflow found
Observer->>Core: run workflow with variables/buttonContent
Core-->>DOM: apply results (keep/clear/sibling/target/order)
else Workflow missing
Observer-->>DOM: insert descriptive missing-workflow block
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/utils/parseSmartBlockButton.ts (1)
40-40: Consider removing redundant ButtonContent injection.Since
buttonContentis already returned as a separate field in the result object, injecting it into thevariablesobject creates redundancy. Based on the AI summary, the consuming code insrc/index.tsnow uses the parsed structure directly.Consider removing this line if the consuming code doesn't require ButtonContent in the variables:
- variables["ButtonContent"] = buttonContent;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/060-alternative-methods.md(1 hunks)src/index.ts(3 hunks)src/utils/parseSmartBlockButton.ts(1 hunks)tests/buttonParsing.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
tests/buttonParsing.test.ts (1)
src/utils/parseSmartBlockButton.ts (1)
parseSmartBlockButton(1-49)
src/index.ts (1)
src/utils/parseSmartBlockButton.ts (1)
parseSmartBlockButton(1-49)
🪛 ast-grep (0.38.6)
src/utils/parseSmartBlockButton.ts
[warning] 15-17: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(
{{(${trimmedLabel.replace(/\\+/g, "\\+")}):(?:42)?SmartBlock:(.*?)}}
)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (6)
docs/060-alternative-methods.md (1)
40-41: LGTM! Clear documentation of the new unlabeled button feature.The documentation clearly explains how to create buttons without captions by omitting the first parameter. The example is helpful and the placement within the existing section flows well.
src/utils/parseSmartBlockButton.ts (1)
30-39: LGTM! Robust argument parsing with proper comma escaping.The argument parsing logic correctly handles commas inside
[[...]]and<%...%>tokens by temporarily escaping them. This prevents incorrect splitting on commas that are part of values rather than delimiters.src/index.ts (3)
49-49: LGTM! Clean import of the new parser utility.The import statement properly brings in the new parseSmartBlockButton function to replace the previous regex-based parsing approach.
549-552: LGTM! Clean integration of the new parser.The refactoring from regex-based parsing to the dedicated parser is well-implemented. Using
textContent.trim()as the label parameter properly handles both labeled and unlabeled buttons.
753-753: Relaxed registration logic is safeThe new
!b.hasAttribute("data-roamjs-smartblock-button")check simply prevents duplicate processing—all actual trigger registration still happens insideregisterElAsSmartBlockTrigger, which callsparseSmartBlockButtonand only attaches listeners when the button’s label and block text match a SmartBlock pattern. Non-SmartBlock BP3 buttons are still ignored.No further changes needed.
tests/buttonParsing.test.ts (1)
1-68: LGTM! Comprehensive test coverage for the new parser.The test suite thoroughly covers the key scenarios for SmartBlock button parsing:
- Unlabeled buttons (the new feature)
- Variable parsing with complex values
- Workflow names containing spaces
- Sibling directives
- Special cases like today's entry
All tests properly verify the parser's output structure including
index,full,buttonContent,workflowName, andvariablesfields.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
- Simplified margin assignment for icons based on text content presence. - Consolidated margin styles into single lines for better readability.
1 participant
Testing
npm testhttps://chatgpt.com/codex/tasks/task_e_689787e1076083269001fdc1aec90bf9
Summary by CodeRabbit
New Features
Documentation
Refactor
Tests
Chores