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

Button, TextLink: Add support for trailing icons #1242

Merged
merged 5 commits into from Mar 8, 2023

Conversation

michaeltaranto
Copy link
Contributor

Provide support for choosing the position of the icon slot within a Button or TextLink.

By default, the iconPosition will be leading the label, but optionally, can be set to trailing.

EXAMPLE USAGE:

<Button
  icon={<IconArrow direction="right" />}
  iconPosition="trailing"
>
  Next
</Button>

@michaeltaranto michaeltaranto requested a review from a team as a code owner March 8, 2023 00:21
@changeset-bot
Copy link

changeset-bot bot commented Mar 8, 2023

🦋 Changeset detected

Latest commit: f31e78f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
braid-design-system Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Collaborator

@mrm007 mrm007 left a comment

Choose a reason for hiding this comment

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

Nice work with the AvoidWidowIcon component 🎩

space?: Space;
}

export const AvoidWidowIcon = ({
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'll batch a few suggestions into one and you can pick and choose what you like:

  • make the children prop more restrictive
  • rename the children prop to icon inside the render function, to make it clear (to humans) what it is
  • move the comment above the HTML entity
import type { UseIconProps } from '../../../hooks/useIcon';
import React from 'react';
import type { Space } from '../../../css/atoms/atoms';
import type { BoxProps } from '../../Box/Box';
import { Box } from '../../Box/Box';

import * as styles from './AvoidWidowIcon.css';

interface Props extends Pick<BoxProps, 'className'> {
  iconPosition: 'leading' | 'trailing';
  space?: Space;
  children: React.ReactElement<UseIconProps>;
}

export const AvoidWidowIcon = ({
  children: icon,
  iconPosition,
  space = 'xxsmall',
  className,
}: Props) => (
  <Box
    component="span"
    paddingRight={iconPosition === 'leading' ? space : undefined}
    paddingLeft={iconPosition === 'trailing' ? space : undefined}
    className={[styles.nowrap, className]}
    aria-hidden
  >
    {iconPosition === 'leading' ? icon : undefined}
    {/* Word joiner character, a zero-width non-breaking character to prevent the icon wrapping onto its own line */}
    &#8288;
    {iconPosition === 'trailing' ? icon : undefined}
  </Box>
);

@michaeltaranto michaeltaranto merged commit f27d41e into master Mar 8, 2023
2 checks passed
@michaeltaranto michaeltaranto deleted the icon-position branch March 8, 2023 06:19
@seek-oss-ci seek-oss-ci mentioned this pull request Mar 8, 2023
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.

None yet

3 participants