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

Add prefixes to all component class names to prevent collisions #90

Merged
merged 1 commit into from
Jun 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 6 additions & 6 deletions src/components/Dialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ export function Dialog({
<div
aria-labelledby={dialogTitleId}
className={classnames(
'Dialog',
{ 'Dialog--closeable': !!onCancel },
'Hyp-Dialog',
{ 'Hyp-Dialog--closeable': !!onCancel },
contentClass
)}
ref={rootEl}
Expand All @@ -112,21 +112,21 @@ export function Dialog({
>
<header>
{icon && (
<div className="Dialog__header-icon">
<div className="Hyp-Dialog__header-icon">
<SvgIcon name={icon} title={title} data-testid="header-icon" />
</div>
)}
<h2 className="Dialog__title" id={dialogTitleId}>
<h2 className="Hyp-Dialog__title" id={dialogTitleId}>
{title}
</h2>
{onCancel && (
<div className="Dialog__close">
<div className="Hyp-Dialog__close">
<IconButton icon="cancel" title="Close" onClick={onCancel} />
</div>
)}
</header>
<div>{children}</div>
<div className="Dialog__actions">
<div className="Hyp-Dialog__actions">
{onCancel && (
<LabeledButton data-testid="cancel-button" onClick={onCancel}>
{cancelLabel}
Expand Down
4 changes: 2 additions & 2 deletions src/components/Modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ export function Modal({ children, onCancel, ...restProps }) {

return (
<>
<div className="Modal__overlay" />
<div className="Modal" ref={modalContainerRef}>
<div className="Hyp-Modal__overlay" />
<div className="Hyp-Modal" ref={modalContainerRef}>
<Dialog onCancel={onCancel} {...restProps}>
{children}
</Dialog>
Expand Down
10 changes: 6 additions & 4 deletions src/components/Panel.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,19 @@ export function Panel({ children, icon, onClose, title }) {
const withCloseButton = !!onClose;
return (
<div
className={classnames('Panel', { 'Panel--closeable': withCloseButton })}
className={classnames('Hyp-Panel', {
'Hyp-Panel--closeable': withCloseButton,
})}
>
<header>
{icon && (
<div className="Panel__header-icon">
<div className="Hyp-Panel__header-icon">
<SvgIcon name={icon} title={title} />
</div>
)}
<h2 className="Panel__title">{title}</h2>
<h2 className="Hyp-Panel__title">{title}</h2>
{withCloseButton && (
<div className="Panel__close">
<div className="Hyp-Panel__close">
<IconButton icon="cancel" title="Close" onClick={onClose} />
</div>
)}
Expand Down
2 changes: 1 addition & 1 deletion src/components/SvgIcon.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export function SvgIcon({ name, className = '', inline = false, title = '' }) {

return (
<span
className={classnames('SvgIcon', { 'SvgIcon--inline': inline })}
className={classnames('Hyp-SvgIcon', { 'Hyp-SvgIcon--inline': inline })}
dangerouslySetInnerHTML={{ __html: markup }}
ref={element}
{...spanProps}
Expand Down
6 changes: 3 additions & 3 deletions src/components/buttons.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ function ButtonBase({
*
* @param {IconButtonProps} props
*/
export function IconButton({ className = 'IconButton', ...restProps }) {
export function IconButton({ className = 'Hyp-IconButton', ...restProps }) {
const { icon } = restProps;
return (
<ButtonBase className={className} {...restProps}>
Expand All @@ -103,7 +103,7 @@ export function IconButton({ className = 'IconButton', ...restProps }) {
*/
export function LabeledButton({
children,
className = 'LabeledButton',
className = 'Hyp-LabeledButton',
...restProps
}) {
const { icon, iconPosition = 'left' } = restProps;
Expand All @@ -122,5 +122,5 @@ export function LabeledButton({
* @param {ButtonBaseProps} props
*/
export function LinkButton(props) {
return <ButtonBase className="LinkButton" {...props} />;
return <ButtonBase className="Hyp-LinkButton" {...props} />;
}
2 changes: 1 addition & 1 deletion src/components/test/Dialog-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe('Dialog', () => {
<span>content</span>
</Dialog>
);
assert.isTrue(wrapper.find('.Dialog').hasClass('foo'));
assert.isTrue(wrapper.find('.Hyp-Dialog').hasClass('foo'));
});

it('renders buttons', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/test/Panel-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ describe('Panel', () => {

it('renders the provided title', () => {
const wrapper = createPanel({ title: 'My Panel' });
const titleEl = wrapper.find('.Panel__title');
const titleEl = wrapper.find('.Hyp-Panel__title');
assert.equal(titleEl.text(), 'My Panel');
});

Expand Down
8 changes: 4 additions & 4 deletions src/components/test/SvgIcon-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,16 +65,16 @@ describe('SvgIcon', () => {
const container = document.createElement('div');
render(<SvgIcon name="arrow-left" />, container);
const wrapper = container.querySelector('span');
assert.isTrue(wrapper.classList.contains('SvgIcon'));
assert.isFalse(wrapper.classList.contains('SvgIcon--inline'));
assert.isTrue(wrapper.classList.contains('Hyp-SvgIcon'));
assert.isFalse(wrapper.classList.contains('Hyp-SvgIcon--inline'));
});

it('appends an inline class to wrapper if `inline` prop is `true`', () => {
const container = document.createElement('div');
render(<SvgIcon name="arrow-left" inline={true} />, container);
const wrapper = container.querySelector('span');
assert.isTrue(wrapper.classList.contains('SvgIcon'));
assert.isTrue(wrapper.classList.contains('SvgIcon--inline'));
assert.isTrue(wrapper.classList.contains('Hyp-SvgIcon'));
assert.isTrue(wrapper.classList.contains('Hyp-SvgIcon--inline'));
});

it('sets a title to the containing `span` element if `title` is present', () => {
Expand Down
23 changes: 9 additions & 14 deletions src/components/test/buttons-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import mockImportedComponents from '../../../test/util/mock-imported-components'

// Add common tests for a button component for stuff provided by `ButtonBase`
function addCommonTests({ componentName, createComponentFn, withIcon = true }) {
const className = `Hyp-${componentName}`;
describe(`${componentName} common support`, () => {
if (withIcon) {
it('renders the indicated icon', () => {
Expand All @@ -17,7 +18,7 @@ function addCommonTests({ componentName, createComponentFn, withIcon = true }) {
const icon = wrapper.find('SvgIcon');
assert.equal(icon.prop('name'), 'fakeIcon');
// Icon is positioned "left" even if it is the only element in the <button>
assert.isTrue(button.hasClass(`${componentName}--icon-left`));
assert.isTrue(button.hasClass(`${className}--icon-left`));
});
}

Expand Down Expand Up @@ -53,43 +54,37 @@ function addCommonTests({ componentName, createComponentFn, withIcon = true }) {
it('uses a default className', () => {
const wrapper = createComponentFn();

assert.isTrue(wrapper.find('button').hasClass(componentName));
assert.isTrue(wrapper.find('button').hasClass(className));
});

['primary', 'light', 'dark'].forEach(variant => {
it('renders a valid variant', () => {
const wrapper = createComponentFn({ variant });

assert.isTrue(
wrapper.find('button').hasClass(`${componentName}--${variant}`)
wrapper.find('button').hasClass(`${className}--${variant}`)
);
});
});

it('sets a `normal` variant modifier class by default', () => {
const wrapper = createComponentFn();

assert.isTrue(
wrapper.find('button').hasClass(`${componentName}--normal`)
);
assert.isTrue(wrapper.find('button').hasClass(`${className}--normal`));
});

['small', 'medium', 'large'].forEach(size => {
it('renders a valid size', () => {
const wrapper = createComponentFn({ size });

assert.isTrue(
wrapper.find('button').hasClass(`${componentName}--${size}`)
);
assert.isTrue(wrapper.find('button').hasClass(`${className}--${size}`));
});
});

it('sets a `medium` size modifier class by default', () => {
const wrapper = createComponentFn();

assert.isTrue(
wrapper.find('button').hasClass(`${componentName}--medium`)
);
assert.isTrue(wrapper.find('button').hasClass(`${className}--medium`));
});

it('overrides className when provided', () => {
Expand All @@ -102,7 +97,7 @@ function addCommonTests({ componentName, createComponentFn, withIcon = true }) {
assert.isTrue(
wrapper.find('button').hasClass('CustomClassName--primary')
);
assert.isFalse(wrapper.find('button').hasClass(componentName));
assert.isFalse(wrapper.find('button').hasClass(className));
});

it('adds inline styles when provided', () => {
Expand Down Expand Up @@ -245,7 +240,7 @@ describe('buttons', () => {
const icon = wrapper.find('SvgIcon');
const button = wrapper.find('button');
assert.equal(icon.prop('name'), 'fakeIcon');
assert.isTrue(button.hasClass('LabeledButton--icon-right'));
assert.isTrue(button.hasClass('Hyp-LabeledButton--icon-right'));
});

it('does not render an icon if none indicated', () => {
Expand Down
29 changes: 15 additions & 14 deletions styles/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,14 @@ Mixins in `mixins` and utility styles in `util` loosely apply [Atomic Design](ht

The directory that a SASS module lives in dictates what it should provide (styles, mixins, variables, etc.) and what kind of dependencies it is allowed:

| Directory | Description | Provides | Dependencies |
| ------------ | ---------------------------- | --------- | --------------------- |
| `base` | Reset and element styles | styles | mixins, variables[^1] |
| `components` | Styles for shared components | styles | any |
| `mixins` | Mixins | mixins | mixins[^2], variables |
| `patterns` | Pattern styles | styles | mixins |
| `util` | Utility styles | styles | mixins |
| `variables` | Variables | variables | none |
| Directory | Description | Provides | Dependencies |
| ------------ | ----------------------------------------------------------- | --------- | --------------------- |
| `base` | Reset and element styles. Used only by the pattern library. | styles | mixins, variables[^1] |
| `components` | Styles for shared components | styles | any |
| `mixins` | Mixins | mixins | mixins[^2], variables |
| `patterns` | Pattern styles | styles | mixins |
| `util` | Utility styles | styles | mixins |
| `variables` | Variables | variables | none |

## CSS Output and Ordering

Expand All @@ -85,13 +85,14 @@ Note `utils` in last position, to assure these classes override other rules.

### Selectors and Class naming

We use [BEM (Block Element Modifier)](http://getbem.com/) methodology for class naming.
All class-name selectors generated by this package's SASS must be prefixed (see details below). We use [BEM (Block Element Modifier)](http://getbem.com/) methodology for class naming.

- `base` modules should use element selectors (no class names).
- Component class names should use PascalCase, e.g. `.HelpPanel`. All other classnames require a .`hyp-` prefix.
- All utility and pattern classnames should be prefixed with `.hyp-`.
- Atomic-level utility classes should be prefixed with `.hyp-u-`, e.g. `.hyp-u-border--left`. These are classes that may be used additively to adjust styling on a single element.
- Composite (molecule, organism) class names do not require `u`-prefixes, but should be lower-case, e.g. `.hyp-frame`.
_Note_: `base` modules use element selectors (no class names). These element selectors and resets are used by the pattern library internally and are not currently part of the public API.

- Component class names should be prefixed with `.Hyp-` and use PascalCase, e.g. `.Hyp-SomePanel`.
- All other class names should be kebab-case:
- Pattern (module, organism, etc.) class names should be prefixed with `.hyp-`, e.g. `.hyp-card`.
- Utility class names should be prefixed with `.hyp-u-`, e.g. `.hyp-u-border--left`. These are classes that may be used individually to adjust styling on a single element.

### Variables

Expand Down
2 changes: 1 addition & 1 deletion styles/components/Dialog.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
@use '../mixins/patterns/organisms';

.Dialog {
.Hyp-Dialog {
@include organisms.panel;
}
4 changes: 2 additions & 2 deletions styles/components/Modal.scss
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
@use '../mixins/layout';
@use '../mixins/patterns/molecules';

.Modal__overlay {
.Hyp-Modal__overlay {
@include layout.overlay;
}

.Modal {
.Hyp-Modal {
@include molecules.modal;
}
2 changes: 1 addition & 1 deletion styles/components/Panel.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
@use '../mixins/patterns/organisms';

.Panel {
.Hyp-Panel {
@include organisms.panel;
}
2 changes: 1 addition & 1 deletion styles/components/SvgIcon.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* Make the wrapper element's size match the contained `svg` element */
.SvgIcon {
.Hyp-SvgIcon {
display: flex;

&--inline {
Expand Down
6 changes: 3 additions & 3 deletions styles/components/buttons/_styles.scss
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
@use 'mixins';

// A button with text, and optionally an icon
.LabeledButton {
.Hyp-LabeledButton {
@include mixins.LabeledButton;
}

// A button with only an icon and no label/text
.IconButton {
.Hyp-IconButton {
@include mixins.IconButton;
}

// A button styled to appear as a link, with underline on hover
.LinkButton {
.Hyp-LinkButton {
@include mixins.LinkButton;
}