feat: Add grouped lines animation mode#459
Conversation
There was a problem hiding this comment.
Creative idea!
One thought I had --
As this is likely an obscure feature, I think it would be better not to have it cause large gaps between line elements by default, though I do like the clean UI implementation.
I noticed that this feature is independent of whether multiline is true or false. Perhaps by default the feature is hidden and the grouping feature could be enabled by selecting a menu item like "Create line groups" from the dropdown?
This would deviate a bit from being a direct adaptation of the query parameters though. Open to suggestions.
Don't need to worry about the formatting check as it should be able to fix itself.
There was a problem hiding this comment.
Pull request overview
Adds a groups parameter so README typing animations can type multiple lines sequentially within a group, clear the group together, and then continue to the next group. The PR also exposes the option in the demo UI and documents/tests the new behavior.
Changes:
- Adds backend parsing, validation, rendering, and randomization behavior for grouped line animations.
- Adds demo UI dividers for creating/restoring line groups in permalinks.
- Adds grouped animation tests and README option documentation.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/models/RendererModel.php |
Adds groups validation and group-preserving random shuffle. |
src/views/RendererView.php |
Passes groups from the model into the SVG template. |
src/templates/main.php |
Adds grouped SMIL animation timing/rendering branch. |
src/demo/js/script.js |
Adds divider UI, URL serialization/restoration, and reset behavior for groups. |
src/demo/css/style.css |
Styles interactive group dividers. |
tests/RendererTest.php |
Adds grouped rendering and random/groups tests. |
tests/OptionsTest.php |
Adds groups option validation tests. |
README.md |
Documents the new groups parameter. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| divider.addEventListener("click", function () { | ||
| this.classList.toggle("active"); | ||
| }); |
| // Lines within each group share the same dur, so groups stay as units | ||
| // Both groups have size 2 with same timing, verify grouped animation is used | ||
| $this->assertStringContainsString("Grouped lines", $svg); |
| | `duration` | Duration of the printing of a single line in milliseconds (default: `5000`) | integer | Any positive number | | ||
| | `pause` | Duration of the pause between lines in milliseconds (default: `0`) | integer | Any non-negative number | | ||
| | `repeat` | `true` to loop around to the first line after the last (default: `true`) | boolean | `true` or `false` | | ||
| | `groups` | Group lines so each group types sequentially then clears together (default: `none`) | string | `2,1` (first 2 lines as group 1, next 1 as group 2) | |
| // DOM layout: [Line 1] [divider after=1] [Line 2] [divider after=2] [Line 3] | ||
| // Remove the divider before the deleted line (index-1), except line 1 | ||
| // has no preceding divider, so remove the one after it instead | ||
| const dividerToRemove = index > 1 ? index - 1 : 1; | ||
| const divider = parent.querySelector(`.group-divider[data-divider-after="${dividerToRemove}"]`); | ||
| if (divider) parent.removeChild(divider); |
Thanks, that makes sense and I agree the default spacing shouldn't be there for everyone.
I'd like to avoid the dropdown mode item if we can, mainly because it'd be the first demo control that doesn't map directly to a query parameter. Everything else in the form is a one-to-one mirror of a param, and a "Create line groups" item would be UI-only state with no URL equivalent. Instead I'd propose keeping the dividers always present but fully collapsed by default. Zero gap and invisible until a divider is actually activated, at which point the gap appears only around the active boundaries. That removes the large default spacing you saw, keeps the demo a faithful mirror of the params (no mode toggle), and the grouping still serializes straight to I will do this approach and then represent the results in this PR, to see what you think @DenverCoder1 👍 |
composer format runs prettier with print width 120 which may account for some differences
display:none until hover/focus-within keeps default line spacing; dividers are <button> with aria-pressed for keyboard use
removeLine dropped the active divider before the deleted line; surviving divider now stays active if either adjacent boundary was
seed RNG and assert intra-group adjacency so an individual-line shuffle regression is caught
Summary
Adds a
groupsparameter that lets users group lines so each group types sequentially, then all lines in the group clear together before the next group starts. For example,groups=2,1means the first two lines type and clear as a unit, then the third line plays solo.Why this change?
I noticed when setting this up for myself, i had long lines of "Quotes" that didn't work as well for longer lines. If i wanted to split it across two lines, it would also spawn the third line when
multilinewas used.This change allows you to use both.
The change can be tested here:
groupsparam onRendererModel, validated so group sizes must sum to total line count.RendererViewandmain.phptemplate compute per-group SMIL animation timings (staggeredbegin, shared clear points).random=trueis used with groups, whole groups are shuffled as units so lines within each group stay together.groupsto the README options table.Note
Some CSS, JS, and PHP files have larger diffs than expected. I ran
composer run formatas described in CONTRIBUTING.md and Prettier decided to reformat a bunch of existing code (wrapping changes, trailing commas, comment repositioning, etc.). I left the reformatting in since fighting the formatter felt worse than including it. Here's what it looks like when you run it:If you'd prefer I separate the formatting into its own commit or revert it entirely, I'm happy to do that. I could not figure out a clean way to get Prettier to only touch the lines I changed. If there's a trick for that, let me know because I genuinely could not find one.
Type of change
How Has This Been Tested?
composer testTest cases added for grouped lines:
testValidGroups,testGroupsNotProvided,testGroupsSumMismatch,testGroupsZeroSize,testGroupedLinesRender,testGroupedLinesPause,testGroupedLinesRepeatFalse,testRandomWithGroups. All pass (47 total, 1 Google Font test failed due to local networking).