refactor(js-example): simplify the management of buttons #822
Conversation
Directly define the buttons in the page instead of using DomHelpers functions to simplify the implementation. This also reduces the size of the application. Improve the script building all examples - list the size of all examples and add an option to only list size without building - move it to the scripts folder for consistency
WalkthroughThis update relocates and enhances the example build script, updates workflow configuration to use the new script path, and modifies the HTML and JavaScript of two example projects to use static buttons for XML actions instead of dynamically creating them. Associated CSS is refactored for clearer structure and scoping of controls. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant index.js
participant GraphModel
User->>Browser: Clicks "View Original XML" button
Browser->>index.js: Triggers click event
index.js->>GraphModel: Retrieves original XML
index.js->>Browser: Displays XML in popup
User->>Browser: Clicks "Export XML" button
Browser->>index.js: Triggers click event
index.js->>GraphModel: Exports current model as XML
index.js->>Browser: Displays exported XML in popup
sequenceDiagram
participant GitHub Actions
participant build-all-examples.bash (old)
participant scripts/build-all-examples.bash (new)
participant ExampleDirs
GitHub Actions->>scripts/build-all-examples.bash (new): Run build script
scripts/build-all-examples.bash (new)->>ExampleDirs: For each example dir, run npm build
scripts/build-all-examples.bash (new)->>ExampleDirs: List JS file sizes in dist/
Possibly related PRs
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 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
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Not used and the current implementation doesn't work on macOS, so remove.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
scripts/build-all-examples.bash (3)
9-12: Enhance argument parsing with usage and help optionCurrently the script only handles
--list-size-onlyand ignores any other flags silently. Consider switching to acase/getopts-based parser and adding a--helpoption to display usage information and exit on unknown arguments.
14-17: Clarify log message when skipping buildThe message
"Skip building examples..."is functional but could be more descriptive. For example:- echo "Skip building examples..." + echo "Skipping build; listing example sizes only..."
43-47: Make file-size listing more robust against spacesParsing
ls -loutput can break on filenames with spaces. You can replace thels+awkpipeline withfind’s-printffor safer, more reliable output:- find "$dir/dist" -name "*.js" -type f -exec ls -l {} \; | LC_NUMERIC=C awk '{ - size_kb = $5 / 1000 - printf "%.2f kB %s\n", size_kb, $9 - }' + find "$dir/dist" -name "*.js" -type f -printf '%s %p\n' | LC_NUMERIC=C awk '{ + size_kb = $1 / 1000 + printf "%.2f kB %s\n", size_kb, $2 + }'.github/workflows/build.yml (1)
59-59: Ensure the build script is executable in CIThe workflow invokes
./scripts/build-all-examples.bashdirectly. Please verify that the script has the executable bit set in the repo, or modify the step to invoke it viabashto avoid permission errors:- run: ./scripts/build-all-examples.bash + run: bash scripts/build-all-examples.bash
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/build.yml(1 hunks)build-all-examples.bash(0 hunks)packages/core/src/util/domHelpers.ts(1 hunks)packages/js-example-selected-features/index.html(1 hunks)packages/js-example-selected-features/src/index.js(1 hunks)packages/js-example-selected-features/src/style.css(1 hunks)packages/js-example/index.html(1 hunks)packages/js-example/src/index.js(1 hunks)packages/js-example/src/style.css(1 hunks)scripts/build-all-examples.bash(1 hunks)
💤 Files with no reviewable changes (1)
- build-all-examples.bash
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/js-example/src/index.js (2)
packages/js-example-selected-features/src/index.js (5)
popup(107-109)xmlWithVerticesAndEdges(46-74)xml(116-116)graph(80-89)graph(104-104)packages/core/src/serialization/ModelXmlSerializer.ts (1)
ModelXmlSerializer(52-73)
🔇 Additional comments (7)
packages/core/src/util/domHelpers.ts (1)
114-124: JSDoc improvements enhance clarity and simplify exampleThe updated documentation for the
buttonfunction now features a more concise description and a streamlined code example. The simplified single-line example better illustrates the typical usage pattern, which aligns with the PR's goal of simplifying button management.packages/js-example-selected-features/index.html (1)
17-20: Good implementation of static buttons with proper accessibility attributesAdding static buttons with descriptive aria-labels is a significant improvement over dynamically created buttons. This approach:
- Makes the UI structure more explicit in HTML
- Improves accessibility with proper aria-labels
- Separates concerns (HTML for structure, JS for behavior)
- Aligns with the PR goal of simplifying button management and reducing application size
The controls container provides a clean way to group related UI elements.
packages/js-example/index.html (1)
17-20: Well-structured static buttons with accessibility supportThe addition of static buttons with descriptive aria-labels is an excellent improvement that:
- Enhances accessibility through proper HTML structure and aria-labels
- Makes the UI more maintainable by keeping structure in HTML
- Follows best practices by separating concerns
- Contributes to the PR's goal of reducing application size by ~3kB
The consistent implementation across example projects shows good attention to maintainability.
packages/js-example/src/style.css (1)
43-49: Better CSS organization with semantic class namesThe CSS refactoring improves the structure by:
- Creating a semantic
.controlsclass that clearly indicates its purpose- Separating container spacing (margin-top) from element spacing (margin-left)
- Following CSS best practices with more specific selectors
This change complements the HTML structure updates and provides better styling organization.
packages/js-example-selected-features/src/index.js (1)
111-118: Cleaner approach using static HTML elementsThis change effectively replaces dynamic button creation with event listeners attached to static HTML elements. The approach is more maintainable as it properly separates concerns between HTML structure and JavaScript behavior.
packages/js-example/src/index.js (1)
87-94: Good separation of concernsThe implementation now attaches event listeners to pre-existing HTML elements rather than creating buttons dynamically. This approach is consistent with the changes in the selected-features example and follows best practices by separating structure (HTML) from behavior (JavaScript).
packages/js-example-selected-features/src/style.css (1)
43-49: Improved CSS organization with proper scopingThe introduction of the
.controlsclass with proper margin spacing and scoped button styling is a good improvement. This change supports the architectural shift to static HTML buttons while maintaining proper styling hierarchy.
|
Directly define the buttons in the page instead of using DomHelpers functions to simplify the implementation.
This also reduces the size of the application (about 3 kB).
Improve the script building all examples
Impact on the size of the examples
The very small decrease in all TS examples and js-example-without-default are due to other PR.
Summary by CodeRabbit
.controlsclass to improve layout and spacing of control buttons in the UI.