-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Alert component #114
Conversation
Create AnimatedExpansion component that handles expand-collapse animation in Expandable, making it possible to easily add this behaviour to other components.
Wrap Alert in AnimatedExpansion component to make it appear and disappear smoothly
const isMounted = useRef(false); | ||
|
||
async function collapseElement() { | ||
await new Promise((resolve) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like something the element-collapse
lib should be doing for us. I'll make a note to see how hard it is to return a promise if a callback wasn't received!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure it is related to your comment but the reason we decided to return a promise here was that we wanted to await the collapse animation to finish before the content collapsing got hidden. This way we'd see the whole animation. See my next comment for a gif.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah no changes needed here nor any problem with your code, I was mostly thinking this is something element-collapse should/could support!
} | ||
|
||
function expandElement() { | ||
expand(expandableRef.current); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the same promise awaiting not done here as collapse?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The promise awaiting in case of collapse
was something me and @digitalsadhu came up with to handle a suddenly disappearing content before the animation is finished (happened in broadcast
web component first):
We only need that for content that gets hidden/removed from DOM after the animation, and not when the content is appearing, like in case of expand
. Does that make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep makes sense!
); | ||
} | ||
|
||
function ExpansionBehaviour({ animated, stateExpanded, children }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
# [1.2.0-next.2](v1.2.0-next.1...v1.2.0-next.2) (2022-06-02) ### Features * **alert:** add expand and collapse behaviour to inline alert ([#114](#114)) ([075f314](075f314))
🎉 This PR is included in version 1.2.0-next.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
* feat(packages): add inline Alert component (#106) Add inline alert component to feedback indicators based on Figma sketches: https://www.figma.com/file/gpHv46zswMNeepUDyQfpS8/Feedback-indicators?node-id=274%3A31588 ### Features * **packages:** add inline Alert component ([#106](#106)) ([ddfe084](ddfe084)) * docs: don't disable button on modal * feat(alert): add expand and collapse behaviour to inline alert (#114) * refactor(expandable): extract transition animation from Expandable Create AnimatedExpansion component that handles expand-collapse animation in Expandable, making it possible to easily add this behaviour to other components. * feat(alert): add expansion and collapsion behaviour to inline alert Wrap Alert in AnimatedExpansion component to make it appear and disappear smoothly * feat(alert): remove neutral variant of inline alert * **alert:** add expand and collapse behaviour to inline alert ([#114](#114)) ([075f314](075f314)) * fix(affix): Add aria-label to Affix icons Screen readers will use the aria-label on the SVG only if the button wrapper has no label. So in that sense, these default labels are like fallbacks. * chore(release): 1.2.0-next.3 [skip ci] # [1.2.0-next.3](v1.2.0-next.2...v1.2.0-next.3) (2022-06-07) ### Bug Fixes * **affix:** Add aria-label to Affix icons ([17fab00](17fab00)) * fix: bump component-classes to get Step fix * chore(deps): change to css/component-classes * fix(animated-expansion): rename to ExpandTransition and check if element ref exists (#119) Fixes an error 'Argument of type 'null' is not assignable to parameter of type 'HTMLElement'. * chore(release): 1.2.0-next.4 [skip ci] # [1.2.0-next.4](v1.2.0-next.3...v1.2.0-next.4) (2022-06-08) ### Bug Fixes * **animated-expansion:** rename to ExpandTransition and check if element ref exists ([#119](#119)) ([b7a8026](b7a8026)) * bump component-classes to get Step fix ([5d99506](5d99506)) * chore(release): 1.2.0-next.5 [skip ci] # [1.2.0-next.5](v1.2.0-next.4...v1.2.0-next.5) (2022-06-08) ### Bug Fixes * bump component-classes to get Step fix ([c17ce7e](c17ce7e)) Co-authored-by: semantic-release-bot <semantic-release-bot@martynus.net> Co-authored-by: Richard Walker <digitalsadhu@gmail.com> Co-authored-by: Martin Storsletten <martin.storsletten@finn.no> Co-authored-by: Benjamin Akar <benjaminakar2001@gmail.com> Co-authored-by: Dave Honneffer <pearofducks@gmail.com>
# [1.2.0](v1.1.1...v1.2.0) (2022-06-08) ### Bug Fixes * **affix:** Add aria-label to Affix icons ([17fab00](17fab00)) * **animated-expansion:** rename to ExpandTransition and check if element ref exists ([#119](#119)) ([b7a8026](b7a8026)) * bump component-classes to get Step fix ([5d99506](5d99506)) * bump component-classes to get Step fix ([c17ce7e](c17ce7e)) ### Features * **alert:** add expand and collapse behaviour to inline alert ([#114](#114)) ([075f314](075f314)) * **packages:** add inline Alert component ([#106](#106)) ([ddfe084](ddfe084)) ### Reverts * Revert "Release from next to main (#120)" (#121) ([04a1976](04a1976)), closes [#120](#120) [#121](#121)
🎉 This PR is included in version 1.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Fixes FRON-593
Description
Inline alert should appear and disappear smoothly based on what state it's in. For this to be possible
Alert
was wrapped in a component calledAnimatedExpansion
(feel free to suggest a better name 😄) that is responsible for handling just the animated behaviour. It was derived from a more complexExpandable
component.Additionally
Removed the "neutral" type of
Alert
based on input from @adidick.Tests