springload
Add basic styling to the NPM package
Currently when you install the package it doesn't come with any css, so this means the modal doesn't even work as a modal, because it actually uses css transitions to open and close. We don't have any documentation about this either.
We have the example css here which could be added into the npm package.
@katie-day this has to be debated I think. We took the party to not style at all any of the components so it's totally free for the dev to start from scratch if needed. If not, they can still copy our styling from the example. I agree it should be at least mentionned in the documentation if we don't proceed to add the styling directly in the package.
@thibaudcolas or @samanthaksanders might be able to tell us more about why we went down that path?
@vincentaudebert I don't think we thought this through much to be honest. Not having bundled styles makes it easier to start from scratch, but it would be just as simple if the styles were optional.
IMHO, at a minimum there should be a line in the README addressing that, like what Quicktube does: https://github.com/springload/quicktube.
Ideally, it would be even better if basic styles were available with the package – and then people can choose whether to import them in their project, or not.
Mmmmmh good point @thibaudcolas as long as importing the styles is optional then why not :)
As @katie-day points out, some of the styles are essential for the modal to work since it's setting up a bunch of animationend listeners
https://github.com/springload/react-accessible-modal/blob/master/lib/index.js#L100
I reckon either the styles need to be added or this.afterOpen etc should be called right away if there's no animation attached. Not sure how to detect that though 🤔
Yeah I didn't realise the code was actually broken without the CSS. Let's just add it to package and mention in the README to not forget to add it. As it is done for Quicktube. I'll do the same on react-accessible-accordion
@mr-winter well the code could use window.getComputedStyle to detect any transition property (and WebkitTransition, MozTransition etc.).
However even if transition is set then if a parent node has display:none then the transitionEnd event also won't fire, so to make transition code robust then it's often necessary to do something like a setTimeout() as a fallback.
E.g. a basic code pattern of
addEventListener('transitionEnd', someCallback);
const timer = setTimeout(someCallback, 3000);
function someCallback() {
if(timer) clearTimeout(timer);
// stuff goes here
}
That way if the transitionEnd is called first it clears the setTimeout timer, but it's still there as a fallback if the user has some weird CSS that conflicts with that event.
(edit: whoops just noticed that you said animationend but the same constraints apply)
