Skip to content

Commit

Permalink
Prefix all component classes with .Hyp-
Browse files Browse the repository at this point in the history
Currently, the SASS entrypoint for this package generates all styles for
all shared components. While we may wish to adapt that piece of the
puzzle at some point to give more granular control, some protection is
needed in the short term at the very least to protect against CSS
conflicts for generically-named components like `Panel`, `Modal`,
`Dialog`, etc., for applications that use this package but might not
yet actually use those individual components (i.e. have implementations
of their own at present).

Utility and pattern classes (which are not currently part of the public
API but will and may be, respectively) are already prefixed with
`.hyp-`.

Adhering to the same general naming principle, these changes add a
`.Hyp-` prefix to all component classes, resulting in, e.g.
`.Hyp-Panel`. Use of these classes is currently an internal concern
only, so these changes should have no effect for consuming applications.
Customization of common component styling,  such as it has been done
thus far, uses mixins and doesn't directly reference class names.

This change should also make it easier to spot whether a component is
common or local to the application.
  • Loading branch information
lyzadanger committed Jun 7, 2021
1 parent b394593 commit aac81bf
Show file tree
Hide file tree
Showing 15 changed files with 56 additions and 58 deletions.
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;
}

0 comments on commit aac81bf

Please sign in to comment.