Skip to content
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

feat(Actions): Add Actions component #139

Merged
merged 3 commits into from
Mar 15, 2019
Merged

feat(Actions): Add Actions component #139

merged 3 commits into from
Mar 15, 2019

Conversation

markdalgleish
Copy link
Member

@markdalgleish markdalgleish commented Mar 14, 2019

This PR adds the initial Actions component (formerly known as ButtonGroup), for rendering a responsive group of buttons/links at the bottom of a form/card/etc.

This PR also makes the following changes:

  • Introduces a responsive flexDirection prop to Box, e.g. <Box flexDirection={['column', 'row']}>
  • Fixes a bug with none values for margin and padding. The corresponding values in the CSS were incorrectly set to none rather than 0.

Example usage

<Actions>
  <Button weight="strong">Submit</Button>
  <TextLink href="#">Cancel</TextLink>
<Actions>

Mobile

Screen Shot 2019-03-15 at 10 07 55 am

Desktop

Screen Shot 2019-03-15 at 10 07 28 am

Development Notes

Internally, we've made the following changes.

  • Moved box-sizing: border-box from Box into our CSS reset, so that link elements rendered by TextLinkRenderer use the correct box model.
  • Introduced a new private component called Overlay to centralise rules for absolutely positioned cover elements, as well as a FieldOverlay component to centralise form overlay rules. This was done to ensure that the focus styles are consistent between TextLinkRenderer and Button, but the idea is that should expand this pattern to other fields going forward.
  • Limited all transition atoms to transform and opacity.

@markdalgleish markdalgleish requested a review from a team as a code owner March 14, 2019 23:11
atoms.color.link,
];

if (inline) {
Copy link
Contributor

@michaeltaranto michaeltaranto Mar 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on hoisting this before the ActionsConsumer? If it's inline it has no contextual behaviour so can return fast.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, nice idea 👍

<span className={styles.overlayContainer}>
{children({
style: {},
className: classnames(defaultStyles, touchableStyles, [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for having an array as the third parameter here, rather than flat parameters?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More good ideas 👍

@@ -1,5 +1,7 @@
export default () => ({
'.transition_fast': { transition: 'all .125s ease' },
'.transition_fast': {
transition: 'transform .125s ease, opacity .125s ease',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💪

@michaeltaranto michaeltaranto merged commit db6f79e into master Mar 15, 2019
@michaeltaranto michaeltaranto deleted the add-actions branch March 15, 2019 03:23
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