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.

Sign up for GitHub

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

NEW Add transparent background state for Badge component #817

Closed
wants to merge 1 commit into from
Closed

NEW Add transparent background state for Badge component #817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/dist/js/bundle.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion client/dist/styles/bundle.css
View file
Open in desktop

Large diffs are not rendered by default.

11 changes: 7 additions & 4 deletions client/src/components/Badge/Badge.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,19 @@ export const statuses = [

class Badge extends PureComponent {
render() {
const { status, inverted, className, message } = this.props;
const { status, inverted, stateBadge, className, message } = this.props;
if (!status) {
return null;
}

const invertedClass = inverted ? `badge-${status}--inverted` : '';

const compiledClassNames = classnames(
className,
'badge',
`badge-${status}`,
invertedClass,
{
[`badge-${status}--inverted`]: inverted,
'badge--state': stateBadge,
}
);
return (
<span className={compiledClassNames}>
Expand All @@ -44,12 +45,14 @@ Badge.propTypes = {
status: PropTypes.oneOf(statuses),
className: PropTypes.string,
inverted: PropTypes.bool,
stateBadge: PropTypes.bool,
};

Badge.defaultProps = {
status: 'default',
className: 'badge-pill',
inverted: false,
stateBadge: false,
};

export default Badge;
7 changes: 7 additions & 0 deletions client/src/components/Badge/Badge.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,10 @@
@include badge-variant-inverted($value);
}
}

// Versioning state badges
.badge--state {
background-color: transparent;
padding-left: 0;
padding-right: 0;
}
3 changes: 3 additions & 0 deletions client/src/components/Badge/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ const props = {
status: 'success',
message: 'The save was successful.',
className: 'action__result',
inverted: false,
stateBadge: false,
};
<Badge {...props} />
```
Expand All @@ -18,3 +20,4 @@ const props = {
* `message` (string): The string to display in the badge.
* `className` (string): Any extra classes to apply for the badge.
* `inverted` (boolean): If the colours should be inverted.
* `stateBadge` (boolean): For use in the context of versioning states (use with inverted).
Copy link
Contributor

Choose a reason for hiding this comment

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

It's nice to know in what context this is used, but I would rather know what it actually does.

Copy link
Contributor Author

@robbieaverill robbieaverill Mar 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah fair call. I'm not super happy about the way it's ending up either - would love a suggestion!

Basically inverted flips the background and text colours, and stateBadge or whatever you want to call it will remove the left and right padding from it, and make the background transparent, so it can be used for versioning state badges e.g. "LIVE", "DRAFT" etc. It's used in combination with inverted so you can get the right text colour, because the OOB badge component usually uses black text.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about unboxed or no-box?

Copy link
Contributor

Choose a reason for hiding this comment

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

I try to avoid giving things names based on appearance as that might change over time, what happens if we designers add a background colour and padding back as we often want to evolve the design over time.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we are better off keeping the name "state" as that shouldn't change over time were as its appearance will. I don't think we need to mention "Badge" again in "stateBadge" (maybe its required so it doesn't conflict with another class).

Copy link
Contributor

Choose a reason for hiding this comment

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

state is very ambiguous in the context of a react component because you might be talking to the component's internal state or a redux state.

status or isStatusBadge might be better. Or maybe we create a <StateBade /> that wrap around the <Badge/> component.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps that's the way to go here. <VersionedStateBadge /> could live in silverstripe/versioned-admin

Copy link
Contributor

Choose a reason for hiding this comment

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

Is versioned-admin a dependency of campaign and CMS? If not you might be introducing some additional coupling.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair point... which would auto enable history viewer. Maybe I’ll just put it in admin but as a new component

2 changes: 1 addition & 1 deletion client/src/components/Badge/tests/Badge-story.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import JSXAddon from 'storybook-addon-jsx';

import Badge, { statuses } from 'components/Badge/Badge';


setAddon(JSXAddon);

storiesOf('Admin/Badges', module)
Expand All @@ -18,5 +17,6 @@ storiesOf('Admin/Badges', module)
status={select('status', statuses, 'default')}
className={boolean('pill', true) ? 'badge-pill' : ''}
inverted={boolean('inverted')}
stateBadge={boolean('stateBadge')}
/>
));