Skip to content

Change triggerAnimation() to animate()#9

Closed
elizabetdev wants to merge 1 commit intonotification_header_buttonfrom
button-animation-proposal
Closed

Change triggerAnimation() to animate()#9
elizabetdev wants to merge 1 commit intonotification_header_buttonfrom
button-animation-proposal

Conversation

@elizabetdev
Copy link
Owner

Hi @Chandler,

When we are in a parent component and we trigger the animation we would call animate():

export default () => {
  const headerUpdatesRef = useRef();

  return (
    <>
      <EuiButton size="s" onClick={() => headerUpdatesRef.current?.animate()}>
        Animate
      </EuiButton>
      <EuiSpacer />
      <EuiHeader>
        <EuiHeaderSection grow={false}>
          <EuiHeaderSectionItem border="right">
            <EuiHeaderLogo>Elastic</EuiHeaderLogo>
          </EuiHeaderSectionItem>
        </EuiHeaderSection>
        <EuiHeaderSection side="right">
          <EuiHeaderSectionItem>
            <HeaderUpdates ref={headerUpdatesRef} />
          </EuiHeaderSectionItem>
        </EuiHeaderSection>
      </EuiHeader>
    </>
  );
};

Then inside of <HeaderUpdates /> component, we would call triggerAnimation():

bellRef.current?.triggerAnimation();

But if we don't have the <HeaderUpdates /> component. We would only call triggerAnimation():

export default () => {
  const bellRef = useRef();

  return (
    <>
      <EuiButton size="s" onClick={() => bellRef.current?.triggerAnimation()}>
        Animate
      </EuiButton>
      <EuiSpacer />
      <EuiHeader>
        <EuiHeaderSection grow={false}>
          <EuiHeaderSectionItem border="right">
            <EuiHeaderLogo>Elastic</EuiHeaderLogo>
          </EuiHeaderSectionItem>
        </EuiHeaderSection>
        <EuiHeaderSection side="right">
          <EuiHeaderSectionItem>
            <EuiHeaderSectionItemButton ref={bellRef}>
              <EuiIcon type="bell" />
            </EuiHeaderSectionItemButton>
          </EuiHeaderSectionItem>
        </EuiHeaderSection>
      </EuiHeader>
    </>
  );
};

It means that we can call triggerAnimation() or animate(). I think this for the user is a little bit confusing. So my proposal is to rename the triggerAnimation() to animate().

So no matter which scenario we are in, we always call animate() to trigger the animation.

What do you think?

@chandlerprall
Copy link

👍 to making the two method names the same. We shouldn't use animate here case because that is already a method on a button element, and we'll want to be additive - and probably prefix with eui to avoid conflicting with other current or future values. Maybe euiAnimate ?

@elizabetdev
Copy link
Owner Author

Closing this because I renamed it directly into notification_header_button branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants