Terra-core: [terra-alert] Uplift to the controlled responsive element

Created on 13 Nov 2019  路  4Comments  路  Source: cerner/terra-core

Feature Request

Investigate uplifting the terra-alert onto the controlled responsive element or alternative.

Description

The uncontrolled responsive element has been flagged for deprecation. The terra-alert should either be uplifting onto the controlled responsive element implementation or alternatively the responsive element should be removed from the component.

terra-alert

Most helpful comment

I brought this up for discussion during the war room. The outcome of those discussions is that the alert should render nothing until a breakpoint is defined.

Displaying nothing and then rendering something on mount tends to be less distracting/jarring than rendering something and then switching to something else. This is the behavior of the previous uncontrolled implementation as well. In the long run we may investigate further into our responsive design strategy.

Here's an example:

const [breakpoint, setBreakpoint] = useState(); // No default

return (
  <ResponsiveElement onChange={value => setBreakpoint(value)}>
    {breakpoint // Check for a breakpoint before rendering the alert
      && (
        <div {...attributes} className={cx('dynamic-class-names-by-breakpoint')}>
          <div className={cx('dynamic-class-names-by-breakpoint')}>
            {getAlertIcon(type, customIcon)}
            {alertMessageContent}
          </div>
          {actionsSection}
        </div>
      )}
  </ResponsiveElement>
);

cc: @pranav300

All 4 comments

Investigation:
I have tried to uplift terra-alert to use controlled responsive-element, which is similar to the examples for responsive-element on the devsite. I have created an internal state breakpoint, which gets updated when the onChange event is triggered. Then inside ResponsiveElement we call a function getSize(Name is random for now). In the getSize function I have kept a check for, breakpoint === 'tiny', based on this alert is toggled between narrow and wide variant.
What do you think of it @StephenEsser and am I on the right track?

Code

Click ME.

  const [breakpoint, setBreakpoint] = useState();

  const getSize = () => {
    if (breakpoint === 'tiny') {
      return (
        <div {...attributes} className={narrowAlertClassNames}>
          <div className={bodyClassNameForNarrowParent}>
            {getAlertIcon(type, customIcon)}
            {alertMessageContent}
          </div>
          {actionsSection}
        </div>
      );
    }
    return (
      <div {...attributes} className={wideAlertClassNames}>
        <div className={cx(['body', 'body-std'])}>
          {getAlertIcon(type, customIcon)}
          {alertMessageContent}
        </div>
        {actionsSection}
      </div>
    );
  };

  return (
    <ResponsiveElement
      onChange={value => setBreakpoint(value)}
    >
      {getSize()}
    </ResponsiveElement>

  );

That looks like you're on the right track. Did you notice any flashing when the component first mounts? Because the component needs to render something initially before updating to the appropriate size I am curious if there is any noticeable dom flashing.

For example if the component defaults to a narrow layout it will display narrow on component did mount but then immediately update to the wide layout if there is enough space. This change should happen very quickly, but may or may not be noticeable to users.

The code is nearly identical between breakpoints. It might be possible to do something inline and dynamically assign classnames.

const [breakpoint, setBreakpoint] = useState();

return (
  <ResponsiveElement onChange={value => setBreakpoint(value)}>
    <div {...attributes} className={cx('dynamic-class-names-by-breakpoint')}>
      <div className={cx('dynamic-class-names-by-breakpoint')}>
        {getAlertIcon(type, customIcon)}
        {alertMessageContent}
      </div>
      {actionsSection}
    </div>
  </ResponsiveElement>
);

I have a draft PR out to get a deployment out - it'd be great if you could check it out and let me know if there's something more that I need to handle. And I am seeing Dom flashing and it is noticeable.

Deployment (color changes after 3 seconds):

https://terra-core-deployed-pr-2747.herokuapp.com/tests/terra-alert/alert/alert-responsive-to-parent

Should we be looking for an alternative for this? @StephenEsser
And I am seeing this behavior in controlled responsive-element as well.

I brought this up for discussion during the war room. The outcome of those discussions is that the alert should render nothing until a breakpoint is defined.

Displaying nothing and then rendering something on mount tends to be less distracting/jarring than rendering something and then switching to something else. This is the behavior of the previous uncontrolled implementation as well. In the long run we may investigate further into our responsive design strategy.

Here's an example:

const [breakpoint, setBreakpoint] = useState(); // No default

return (
  <ResponsiveElement onChange={value => setBreakpoint(value)}>
    {breakpoint // Check for a breakpoint before rendering the alert
      && (
        <div {...attributes} className={cx('dynamic-class-names-by-breakpoint')}>
          <div className={cx('dynamic-class-names-by-breakpoint')}>
            {getAlertIcon(type, customIcon)}
            {alertMessageContent}
          </div>
          {actionsSection}
        </div>
      )}
  </ResponsiveElement>
);

cc: @pranav300

Was this page helpful?
0 / 5 - 0 ratings