Quality of life updates#103
Conversation
Replace the built-in SVG foil overlay (data-foil) with CSS custom properties (--perspective-card-angle, --perspective-card-tilt) so consumers can drive their own per-frame effects from pure CSS. Route all ClickablePerspectiveCard activation through a real <button> element (auto-created if absent), removing the pointer/touch juggling in favour of a single click listener. Adds aria-haspopup, aria-expanded, keyboard open/close, and focus-return on close. Update README, USAGE, and demo with an Accessibility section, CSS custom property docs, and a holographic foil recipe.
…ult. Supports custom aria copy by provision of data-close-button-label (defaulting to "Close"
…logue and abstracting a modal cleanup function to be used both in this case and in the regular close case
…ting the usage documentation to remove reference to decoration (old and redundant)
…f the card changes between collapsed and animating state (and vice versa) due to the collapsed state card being fluid. Added dimension copy and resize observer to address.
liamegan
requested review from
consolecmnd,
meteoratlas,
pravton and
saugatmalla
There was a problem hiding this comment.
We should probably just delete these.
There was a problem hiding this comment.
Yeah, or if we want to keep it let's be clear about it, what do you think?
- rename to index.js
- re-export directly:
export { PerspectiveCard } from './perspective-card.js'
export { ClickablePerspectiveCard } from './clickable-perspective-card.js'
There was a problem hiding this comment.
Pull request overview
This PR modernizes wtc-perspective-card with a Vite-based build, splits the core classes into dedicated modules, and reworks the clickable/modal experience to use <dialog> with improved accessibility and new lifecycle events.
Changes:
- Replace the legacy Webpack/Babel build with a Vite library build and add a demo app build.
- Refactor
PerspectiveCard/ClickablePerspectiveCardinto separate source modules, add CSS custom properties for tilt/angle, and dispatch custom events (perspectivecard:*). - Redesign the clickable/modal behavior around the native
<dialog>element, with an accessible trigger<button>, focus management, and optional injected close button.
Reviewed changes
Copilot reviewed 15 out of 22 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| webpack.config.js | Removes the old Webpack build configuration. |
| vite.config.js | Adds Vite config for library build (ES/UMD) and demo build. |
| package.json | Updates scripts/deps for Vite build; bumps major version to 3.0.0. |
| package-lock.json | Lockfile updates for new toolchain (also needs version sync; see comments). |
| src/wtc-perspective-card.js | Becomes a small entrypoint that re-exports the two classes. |
| src/perspective-card.js | Extracted base card logic; adds CSS custom properties + event dispatching. |
| src/clickable-perspective-card.js | New <dialog>-based modal implementation + accessibility improvements + events. |
| src/wtc-perspective-card.scss | Adds button/dialog/close-button styling and dialog backdrop animations. |
| USAGE.md | Expanded docs for attributes, accessibility, CSS custom properties, and events. |
| README.md | Updates generated/combined docs, but currently contains stale _setupFoil API references. |
| demo/index.html | Adds a demo page showcasing hover/ambient/clickable/custom-button/foil examples. |
| demo/main.js | Demo entrypoint wiring up the card controllers. |
| demo/demo.scss | Demo styling and example foil/badge CSS. |
| demo/assets/00-simplest/index.html | Adds an additional demo asset set (appears unused by the demo page). |
| demo/assets/00-simplest/bootloader.js | Bootloader utility for the additional demo asset set. |
| .npmignore | Excludes demo/ from the published package. |
| .gitignore | Ignores demo/dist output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| onDialogCancel(e) { | ||
| // Prevent the native immediate-close so we can run the close animation instead. | ||
| // Note: browsers fire a second, non-cancelable cancel event on repeated Escape | ||
| // presses as a safety valve - preventDefault() will silently fail for those. | ||
| // onDialogClose handles that case. | ||
| e.preventDefault() | ||
| if (this.tweening) { | ||
| // Can't start the close tween yet - record the intent and honour it | ||
| // once the current tween's onEndTween has fully committed its state. | ||
| this._pendingClose = true | ||
| } else { | ||
| this.enlarged = false | ||
| } | ||
| } |
There was a problem hiding this comment.
I think the "immediate close" issue can be handled with some modern CSS. allow-discrete in the CSS should do it. I've also added @starting-style to this snippet so it animates “on open”.
dialog::backdrop {
opacity: 0;
transition: display allow-discrete, overlay allow-discrete, opacity;
transition-duration: 0.4s;
}
dialog:open::backdrop {
opacity: 1;
}
@starting-style {
dialog:open::backdrop {
opacity: 0
}
}There was a problem hiding this comment.
allow-discrete doesn't interact with javascript animations though, does it? The issue here is the order of operations:
- Click the card
- Dialog is added and card is moved there
- Animation up starts
- User hits escape twice, which immediately closes the dialog
- There is a race condition between the end of the card up animation and the close function.
There was a problem hiding this comment.
I think it should, Liam. These are the styles from PCOM, it's simple but has a nice little "lift" add to the content. I think it already does but if it doesn't the dialog should probably be added to the DOM as soon as the class is initialized.
@layer silph-ui {
.modal {
--modal-animation-duration: 0.2s;
--modal-animation-easing: ease-in-out;
background: transparent;
border: none;
block-size: 100%;
margin: 0;
max-block-size: 100%;
max-inline-size: 100%;
inline-size: 100%;
&::backdrop {
background-color: rgb(0 0 0 / 0%);
}
&:open {
&::backdrop {
background-color: rgb(0 0 0 / 70%);
}
.content {
opacity: 1;
transform: translateY(0) scale(1);
}
}
&:not([open]) {
display: none;
}
}
.content {
opacity: 0;
transform: translateY(1em) scale(0.96);
transform-origin: center center;
transition:
opacity var(--modal-animation-duration) var(--modal-animation-easing),
transform var(--modal-animation-duration) var(--modal-animation-easing);
inline-size: 100%;
}
@supports (transition-behavior: allow-discrete) and (transition: display 1s) and (transition: overlay 1s) and (overlay: unset) {
.modal {
transition:
display var(--modal-animation-duration) allow-discrete,
overlay var(--modal-animation-duration) allow-discrete;
transition-behavior: allow-discrete;
&::backdrop {
transition: background-color var(--modal-animation-duration) var(--modal-animation-easing);
}
@starting-style {
&:open {
&::backdrop {
background-color: rgb(0 0 0 / 0%);
}
.content {
opacity: 0;
transform: translateY(1em) scale(0.96);
}
}
}
}
}
}There was a problem hiding this comment.
Hrm I don't think this will work here as we move the card from the place in the dom to the dialog on click (this is necessary to have a seamless transition of the animating cards), so there's not really any reason to persist the dialog in the dom outside of when it's needed, unless we want to try to maintain a single dialog for every card on the page - doable, but a pretty significant rewrite.
Would one of you mind connecting with me for a pair code on this to assess feasibility? I'm having a difficult time understanding it.
| * .perspective-card | ||
| * .perspective-card__transformer | ||
| * .perspective-card__artwork.perspective-card__artwork--front | ||
| * img | ||
| * .perspective-card__artwork.perspective-card__artwork--back | ||
| * img | ||
| * .perspective-card__shine |
There was a problem hiding this comment.
It might be helpful to abstract some of the base classes into constants that can be referred to throughout the JS. Thoughts? Just to avoid typos etc. when maintaining the repo down the road.
| // set settings | ||
| this.debug = 'debug' in settings | ||
| ? settings.debug | ||
| : this.element.hasAttribute('data-debug') | ||
| this.zoomSize = 'zoom' in settings | ||
| ? settings.zoom | ||
| : (parseInt(this.element.getAttribute('data-zoom')) || 40) | ||
| this.intensity = 'intensity' in settings | ||
| ? settings.intensity | ||
| : (parseInt(this.element.getAttribute('data-intensity')) || 10) | ||
|
|
||
| this.ambient = -1 | ||
|
|
||
| if (settings.ambient !== undefined && settings.ambient !== false) { | ||
| const settingsVal = settings.ambient | ||
| if (settingsVal === true) this.ambient = 0 | ||
| else this.ambient = settingsVal | ||
| } else if (this.element.hasAttribute('data-ambient')) { | ||
| const dataVal = this.element.getAttribute('data-ambient') |
There was a problem hiding this comment.
Would it be beneficial to grab this.element.dataset up front, do that you don't have to query the element's attributes each time?
There was a problem hiding this comment.
Good idea, it likely means introducing a parsing function up front and removing a lot of these conditionals.
card.addEventListener('perspectivecard:opened', CALLBACK)It would be very good to test this locally with TCG before merging and publishing. I think next steps here might be turning this into a monorepo and publishing a react version alongside.
To test this locally:
git clone git@github.com:wethegit/wtc-perspective-card.gitAnd then run the dev site, your local dev should now be using the local wtc-perspective-card.