Terra-core: ActionHeader Port from clinical

Created on 4 Mar 2018  路  7Comments  路  Source: cerner/terra-core

Summary

There exists an action header in the terra-clinical repo already. We would like to push this into terra-core and deprecate the clinical package. To make upgrading easiest, we would like to try to maintain the api for passivity.

Existing Props

| Prop | Type | Required | Default | Description
|------|------|-------------|--------|-------------
| title | string | optional | undefined | Text to be displayed as the title in the header bar
|level | number | optional | 1 | Sets the heading level. One of 1, 2, 3, 4, 5, 6. (ADDED)
| onClose | func | optional | undefined | Callback function for when the close button is clicked. The back button will not display if this is not set. On small viewports a back button will be displayed instead of a close button when a separate onBack callback is not set.
| onBack | func | optional | undefined | Callback function for when the back button is clicked. The back button will not display if this is not set.
| onMaximize | func | optional | undefined | Callback function for when the expand button is clicked. The expand button will not display if this is not set or on small viewports. Only the expand button will be rendered if onMaximize and onMinimize are set.
| onMinimize | func | optional | undefined | Callback function for when the minimize button is clicked. The minimize button will not display if this is not set or on small viewports. Only the expand button will be rendered if both onMaximize and onMinimize are set.
| onNext | func | optional | undefined | Callback function for when the next button is clicked. The previous-next button group will not display if neither this or onPrevious are set.
| onPrevious | func | optional | undefined | Callback function for when the previous button is clicked. The previous-next button group will not display if neither this or onNext are set.
| keepCloseButton | bool | optional | undefined | A Boolean indicating if close button should be displayed on small viewports when separate onBack callback is not set.
| children | element | optional | undefined | Child element to be displayed on the right end of the header. This is intended to be used with the CollapsibleMenuView.

Changes

  • Currently the back button, and the expand/collapse buttons appear to be mutually exclusive as they share the same spot and change based off screen size https://github.com/cerner/terra-clinical/blob/master/packages/terra-clinical-action-header/src/ActionHeader.jsx#L129 . They will no longer be mutually exclusive as the back button, and expand/collapse will now be in 2 different sockets.
  • The existing action header uses functions specific to each special button (maximize, minimize, next, previous) to determine if they should be shown. These props will be kept for passivity, but it will be documented that they will be removed in the next major release, and instead the consumer should be passing them into their collapsible menu as children.
  • Button styles have been changed to utility
  • @mjhenkes , we discussed with @neilpfeiffer, and he thought we should put the buttons from legacy function props in their same existing position (between the back button and title) for now.

How to deprecate the props

Seen here, https://discuss.reactjs.org/t/proposal-add-isdeprecated-to-react-proptypes/4037 something like this would work

import React from 'react';

class Component extends React.Component {
  render() {
    const {oldProp, newProp} = this.props;
    if (process.env.NODE_ENV !== 'production' && typeof oldProp !== 'undefined') {
      console.warn('oldProp is deprecated, use newProp instead');
    }
    ...
  }
}

React chose not to add an isDeprecated function for prop-types https://github.com/facebook/react/pull/6357 . Dan Abramov did make a package for this https://github.com/Aweary/react-is-deprecated but its not worth pulling in

Questions

  • Do the buttons' icon need to be themable? I'm assuming atleast some of them do since I see a vertical ... and a horizontal version in different spots.
  • Digging in more now, I see ActionHeader has a dependency on the terra-clinical-header. Is this also a component we want ported over directly (I don't see anything about it in callouts)? I assume we don't want a dependency from core -> clinical. We could probably get most of the functionality we need utilizing arrange, heading, and truncating the text.
terra-action-header

Most helpful comment

Just to clarify, we are NOT planning to create a core version of terra-clinical-header, but rather integrate it as part of the action-header component itself. Is this correct?

All 7 comments

I think we should decouple action-header from the header component, I'm guessing there are going to be theming differences between the two that require them to be decoupled.

Cool. Skimming through how it was used and how the header itself works, it seems like the children (collapsible panel) provided as children were then provided as children to header
https://github.com/cerner/terra-clinical/blob/master/packages/terra-clinical-action-header/src/ActionHeader.jsx#L182
which then adds the flex-collapse class
https://github.com/cerner/terra-clinical/blob/master/packages/terra-clinical-header/src/Header.jsx#L70
which gets a max-width of 40%
https://github.com/cerner/terra-clinical/blob/master/packages/terra-clinical-header/src/Header.scss#L38

So this way the button at the start and end are fixed in size, the collapsible panel has a % based width, and the title fills the rest and trucates
https://github.com/cerner/terra-clinical/blob/master/packages/terra-clinical-header/src/Header.scss#L58

We can try to keep the same requirements

Just to clarify, we are NOT planning to create a core version of terra-clinical-header, but rather integrate it as part of the action-header component itself. Is this correct?

@alex-bezek - I updated the props table above to include a new item level since consumers will need to be able to change up which-ever h1/2/3... level they need for correct screenreader accessibility expectations based on nesting order.

It is based on these comments realized for section-header: https://github.com/cerner/terra-core/pull/1313#issuecomment-370591792 and https://github.com/cerner/terra-core/pull/1313#discussion_r172290552

@ChaseSinclair We are not planning on creating a core version of terra-clinical-header. Anything terra-clinical-header provided terra-clincal-action-header should be coded directly into terra-action-header.

Do the buttons' icon need to be themable?

Yeah, let's assume so and make the icons like the back, close, more, prev, next, etc icons themable.

+1 on tech design.

+1 on the tech design

+1 on tech design

Was this page helpful?
0 / 5 - 0 ratings