Skip to content

Commit

Permalink
Fix ID generation to ensure it produces valid CSS identifiers (#3139)
Browse files Browse the repository at this point in the history
* Move the logic of generating the ID into its own utility
  function.

* Ensure the utility function generates IDs that are valid
  CSS identifiers.

  `shortid.generate()` by itself will generates IDs that
  are not always valid¹ CSS identifiers (e.g. will start
  with a digit). So, when other parts of the codebase will
  use those IDs as a selector, a `SyntaxError`² will be thrown
  (i.e. `'#...' is not a valid selector`).

  The `slds-` prefix is used so to keep things simple.
  `shortid.characters()` requires "a string of all 64 unique
  characters"³, which means that in order to generate valid
  CSS identifiers a-z, A-Z, and some "ISO 10646 characters
  U+00A0 and higher"¹ will need to be included.

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

¹ From https://www.w3.org/TR/CSS21/syndata.html#value-def-identifier

 " In CSS, identifiers (including element names, classes, and IDs
   in selectors) can contain only the characters [a-zA-Z0-9] and
   ISO 10646 characters U+00A0 and higher, plus the hyphen (-) and
   the underscore (_); they cannot start with a digit, two hyphens,
   or a hyphen followed by a digit. "

² https://dom.spec.whatwg.org/#selectors

³ https://github.com/dylang/shortid/blob/937ce2c8dd7001baec1785cb8dce6e6fe1bcf61f/README.md#shortidcharactersstring
  • Loading branch information
alrra authored Mar 1, 2024
1 parent b2ab369 commit fb71cd1
Show file tree
Hide file tree
Showing 51 changed files with 157 additions and 234 deletions.
4 changes: 2 additions & 2 deletions components/accordion/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
import React, { Component } from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';
import shortid from 'shortid';

import { ACCORDION } from '../../utilities/constants';
import generateId from '../../utilities/generate-id';

const propTypes = {
/**
Expand Down Expand Up @@ -49,7 +49,7 @@ class Accordion extends Component {
super(props);
this.state = { currButtonIndex: 0 };
this.summaryButtons = [];
this.generatedId = shortid.generate();
this.generatedId = generateId();
}

componentDidUpdate(prevProps, prevState) {
Expand Down
4 changes: 2 additions & 2 deletions components/app-launcher/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import PropTypes from 'prop-types';
import classNames from 'classnames';

import isFunction from 'lodash.isfunction';
import shortid from 'shortid';

// This component's `checkProps` which issues warnings to developers about properties when in development mode (similar to React's built in development tools)
import checkProps from './check-props';
Expand All @@ -20,6 +19,7 @@ import componentDoc from './component.json';
import Modal from '../modal';

import { APP_LAUNCHER } from '../../utilities/constants';
import generateId from '../../utilities/generate-id';

const defaultProps = {
assistiveText: {
Expand Down Expand Up @@ -127,7 +127,7 @@ class AppLauncher extends React.Component {

constructor(props) {
super(props);
this.generatedId = shortid.generate();
this.generatedId = generateId();
this.state = {
isOpen: false,
};
Expand Down
9 changes: 3 additions & 6 deletions components/badge/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,18 @@ import React from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';

// ### shortid
// [npmjs.com/package/shortid](https://www.npmjs.com/package/shortid)
// shortid is a short, non-sequential, url-friendly, unique id generator
import shortid from 'shortid';

// ## Constants
import { BADGE } from '../../utilities/constants';

import generateId from '../../utilities/generate-id';

/**
* Badges are labels which hold small amounts of information.
*/
class Badge extends React.Component {
constructor(props) {
super(props);
this.generatedId = shortid.generate();
this.generatedId = generateId();
}

/**
Expand Down
9 changes: 3 additions & 6 deletions components/brand-band/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,6 @@
import React from 'react';
import PropTypes from 'prop-types';

// ### shortid
// [npmjs.com/package/shortid](https://www.npmjs.com/package/shortid)
// shortid is a short, non-sequential, url-friendly, unique id generator
import shortid from 'shortid';

// ### classNames
// [github.com/JedWatson/classnames](https://github.com/JedWatson/classnames)
// A simple javascript utility for conditionally joining classNames together.
Expand All @@ -27,6 +22,8 @@ import checkProps from './check-props';
// ## Constants
import { BRAND_BAND } from '../../utilities/constants';

import generateId from '../../utilities/generate-id';

/**
* The brand band provides theming capability that adds personality and improves information density and contrast.
*/
Expand All @@ -35,7 +32,7 @@ class BrandBand extends React.Component {
super(props);

checkProps(BRAND_BAND, this.props);
this.generatedId = shortid.generate();
this.generatedId = generateId();
}

getId() {
Expand Down
8 changes: 2 additions & 6 deletions components/button-group/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,8 @@ import classNames from 'classnames';

import assign from 'lodash.assign';

// ### shortid
// [npmjs.com/package/shortid](https://www.npmjs.com/package/shortid)
// shortid is a short, non-sequential, url-friendly, unique id generator
import shortid from 'shortid';

import { BUTTON_GROUP } from '../../utilities/constants';
import generateId from '../../utilities/generate-id';

const propTypes = {
/**
Expand Down Expand Up @@ -66,7 +62,7 @@ const defaultProps = { labels: {} };
class ButtonGroup extends React.Component {
constructor(props) {
super(props);
this.generatedId = shortid.generate();
this.generatedId = generateId();
}

getId() {
Expand Down
5 changes: 3 additions & 2 deletions components/card/__docs__/storybook-stories.jsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React from 'react';

import PropTypes from 'prop-types';
import shortid from 'shortid';
import { storiesOf } from '@storybook/react';
import { action } from '@storybook/addon-actions';
import IconSettings from '../../icon-settings';
Expand All @@ -21,6 +20,8 @@ import InlineEdit from '../../forms/input/inline';

import RelatedListWithTable from '../__examples__/related-list-with-table';

import generateId from '../../../utilities/generate-id';

const sampleItems = [
{ id: '0', name: 'Cloudhub' },
{ id: '1', name: 'Cloudhub + Anypoint Connectors' },
Expand Down Expand Up @@ -69,7 +70,7 @@ class DemoCard extends React.Component {
this.setState({
items: [
// eslint-disable-next-line no-plusplus
{ id: currentId++, name: `New item #${shortid.generate()}` },
{ id: currentId++, name: `New item #${generateId()}` },
...this.state.items,
],
});
Expand Down
9 changes: 3 additions & 6 deletions components/carousel/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,10 @@ import React from 'react';
import PropTypes from 'prop-types';
import classnames from 'classnames';

// ### shortid
// [npmjs.com/package/shortid](https://www.npmjs.com/package/shortid)
// shortid is a short, non-sequential, url-friendly, unique id generator
import shortid from 'shortid';

import { CAROUSEL } from '../../utilities/constants';

import generateId from '../../utilities/generate-id';

import {
canUseDOM,
canUseEventListeners,
Expand Down Expand Up @@ -151,7 +148,7 @@ class Carousel extends React.Component {
translateX: -1000000,
};

this.generatedId = shortid.generate();
this.generatedId = generateId();
}

componentDidMount() {
Expand Down
8 changes: 2 additions & 6 deletions components/checkbox/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,6 @@ import PropTypes from 'prop-types';

import classNames from 'classnames';

// ### shortid
// [npmjs.com/package/shortid](https://www.npmjs.com/package/shortid)
// shortid is a short, non-sequential, url-friendly, unique id generator
import shortid from 'shortid';

// ### Event Helpers
import KEYS from '../../utilities/key-code';
import EventUtil from '../../utilities/event';
Expand All @@ -28,6 +23,7 @@ import { CHECKBOX } from '../../utilities/constants';
import Icon from '../icon';

import getAriaProps from '../../utilities/get-aria-props';
import generateId from '../../utilities/generate-id';

const propTypes = {
/**
Expand Down Expand Up @@ -211,7 +207,7 @@ class Checkbox extends React.Component {
super(props);

checkProps(CHECKBOX, this.props, componentDoc);
this.generatedId = shortid.generate();
this.generatedId = generateId();
}

getId = () => this.props.id || this.generatedId;
Expand Down
4 changes: 2 additions & 2 deletions components/color-picker/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import classNames from 'classnames';
import PropTypes from 'prop-types';
import React from 'react';
import shortid from 'shortid';
import assign from 'lodash.assign';

import checkProps from './check-props';
Expand All @@ -21,6 +20,7 @@ import Popover from '../popover';
import ColorUtils from '../../utilities/color';

import { COLOR_PICKER } from '../../utilities/constants';
import generateId from '../../utilities/generate-id';

import componentDoc from './component.json';

Expand Down Expand Up @@ -247,7 +247,7 @@ class ColorPicker extends React.Component {
constructor(props) {
super(props);

this.generatedId = props.id || shortid.generate();
this.generatedId = props.id || generateId();
const workingColor = ColorUtils.getNewColor(
{
hex: props.valueWorking || props.value,
Expand Down
7 changes: 3 additions & 4 deletions components/combobox/combobox.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ import isFunction from 'lodash.isfunction';

import classNames from 'classnames';

import shortid from 'shortid';

import Button from '../button';
import Dialog from '../utilities/dialog';
import InnerInput from '../../components/input/private/inner-input';
Expand All @@ -37,6 +35,7 @@ import menuItemSelectScroll from '../../utilities/menu-item-select-scroll';
import checkProps from './check-props';

import { COMBOBOX } from '../../utilities/constants';
import generateId from '../../utilities/generate-id';
import componentDoc from './component.json';
import { IconSettingsContext } from '../icon-settings';

Expand Down Expand Up @@ -446,8 +445,8 @@ class Combobox extends React.Component {
// `checkProps` issues warnings to developers about properties (similar to React's built in development tools)
checkProps(COMBOBOX, props, componentDoc);

this.generatedId = shortid.generate();
this.generatedErrorId = shortid.generate();
this.generatedId = generateId();
this.generatedErrorId = generateId();
this.deselectId = `${this.getId()}-deselect`;
}

Expand Down
11 changes: 11 additions & 0 deletions components/component-docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -5771,6 +5771,17 @@
"computed": false
}
},
"disabledSelection": {
"type": {
"name": "array"
},
"required": false,
"description": "An array of objects of rows that selection is disabled. See `items` prop for shape of objects.",
"defaultValue": {
"value": "[]",
"computed": false
}
},
"selectRows": {
"type": {
"name": "union",
Expand Down
10 changes: 3 additions & 7 deletions components/data-table/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@
import React from 'react';
import PropTypes from 'prop-types';

// ### shortid
// [npmjs.com/package/shortid](https://www.npmjs.com/package/shortid)
// shortid is a short, non-sequential, url-friendly, unique id generator
import shortid from 'shortid';

import classNames from 'classnames';
import assign from 'lodash.assign';
import isEqual from 'lodash.isequal';
Expand Down Expand Up @@ -42,6 +37,7 @@ import Mode from './private/mode';
import Spinner from '../spinner';

import KEYS from '../../utilities/key-code';
import generateId from '../../utilities/generate-id';
import mapKeyEventCallbacks from '../../utilities/key-callbacks';

import {
Expand Down Expand Up @@ -404,7 +400,7 @@ class DataTable extends React.Component {

constructor(props) {
super(props);
this.generatedId = shortid.generate();
this.generatedId = generateId();
this.headerRefs = {
action: [],
column: [],
Expand Down Expand Up @@ -1229,7 +1225,7 @@ class DataTable extends React.Component {
const rowId =
this.getId() && item.id
? `${this.getId()}-${DATA_TABLE_ROW}-${item.id}`
: shortid.generate();
: generateId();
return this.props.onRenderSubHeadingRow &&
item.type === 'header-row' ? (
this.props.onRenderSubHeadingRow({
Expand Down
5 changes: 3 additions & 2 deletions components/data-table/interactive-element.jsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
/* eslint-disable fp/no-rest-parameters */
import React from 'react';
import shortid from 'shortid';
import Mode from './private/mode';
import CellContext from './private/cell-context';
import TableContext from './private/table-context';

import generateId from '../../utilities/generate-id';

/**
* Wrapper for interactive elements in the table.
*
Expand All @@ -21,7 +22,7 @@ export default (WrappedElement) => {
class InteractiveElement extends React.Component {
constructor(props) {
super(props);
this.elementId = shortid.generate();
this.elementId = generateId();
}

onFocus(tableContext, ...args) {
Expand Down
8 changes: 2 additions & 6 deletions components/date-picker/date-picker.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,6 @@ import assign from 'lodash.assign';
// joining classNames together."
import classNames from 'classnames';

// ### shortid
// [npmjs.com/package/shortid](https://www.npmjs.com/package/shortid)
// shortid is a short, non-sequential, url-friendly, unique id generator
import shortid from 'shortid';

import Dialog from '../utilities/dialog';
import CalendarWrapper from './private/calendar-wrapper';
import InputIcon from '../icon/input-icon';
Expand All @@ -33,6 +28,7 @@ import KEYS from '../../utilities/key-code';
import lowPriorityWarning from '../../utilities/warning/low-priority-warning';

import { DATE_PICKER } from '../../utilities/constants';
import generateId from '../../utilities/generate-id';
import { IconSettingsContext } from '../icon-settings';

const propTypes = {
Expand Down Expand Up @@ -273,7 +269,7 @@ class Datepicker extends React.Component {
inputValue: initDate || '',
};

this.generatedId = shortid.generate();
this.generatedId = generateId();

// `checkProps` issues warnings to developers about properties (similar to React's built in development tools)
checkProps(DATE_PICKER, props, componentDoc);
Expand Down
5 changes: 3 additions & 2 deletions components/docked-composer/index.jsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import React from 'react';
import PropTypes from 'prop-types';
import shortid from 'shortid';

import Button from '../button';

import generateId from '../../utilities/generate-id';

const propTypes = {
/**
* HTML id for component.
Expand Down Expand Up @@ -71,7 +72,7 @@ const defaultProps = {
class DockedComposer extends React.Component {
constructor(props) {
super(props);
this.generatedId = shortid.generate();
this.generatedId = generateId();
}

getId() {
Expand Down
Loading

0 comments on commit fb71cd1

Please sign in to comment.