Skip to content

Commit

Permalink
Merge branch 'STCOM-1380' of github.com:folio-org/stripes-components …
Browse files Browse the repository at this point in the history
…into STCOM-1380
  • Loading branch information
JohnC-80 committed Nov 13, 2024
2 parents 36f071c + b3edd21 commit 5cb1421
Show file tree
Hide file tree
Showing 19 changed files with 203 additions and 52 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ui.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
with:
jest-enabled: false
bigtest-enabled: true
bigtest-test-command: xvfb-run --server-args="-screen 0 1024x768x24" yarn test $YARN_TEST_OPTIONS --karma.singleRun --karma.browsers ChromeDocker --karma.reporters mocha junit --coverage
bigtest-test-command: yarn test $YARN_TEST_OPTIONS
sonar-sources: ./hooks,./lib,./util
generate-module-descriptor: false
publish-module-descriptor: false
Expand Down
17 changes: 16 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,21 @@
# Change history for stripes-components

## 12.2.0 IN PROGRESS
## 12.3.0 IN PROGRESS

* `TextArea` - move focus to the field after clearing the field by clicking on the `x` icon. Refs STCOM-1369.
* Change `Repeatable field` focus behaviour. Refs STCOM-1341.
* Fix `<Selection>` bug with option list closing when scrollbar is used. Refs STCOM-1371.
* `<Selection>` - fix bug handling empty string options/values. Refs STCOM-1373.
* Include Kosovo in the countries list. Refs STCOM-1354.
* `<RepeatableField>` - switch to MutationObserver to resolve focus-management issues. Refs STCOM-1372.
* Bump `stripes-react-hotkeys` to `v3.2.0` for compatibility with `findDOMNode()` changes. STCOM-1343.
* Pin `currency-codes` to `v2.1.0` to avoid duplicate entries in `v2.2.0`. Refs STCOM-1379.
* Wrap `<Selection>` in full-width div. Refs STCOM-1332.
* Assign `<Modal>`'s exit key handler to Modal's element rather than `document`. refs STCOM-1382.

## [12.2.0](https://github.com/folio-org/stripes-components/tree/v12.2.0) (2024-10-11)
[Full Changelog](https://github.com/folio-org/stripes-components/compare/v12.1.0...v12.2.0)

* Add specific loading props to MCL to pass to Prev/Next pagination row, Refs STCOM-1305
* Exclude invalid additional currencies. Refs STCOM-1274.
* Validate ref in `Paneset` before dereferencing it. Refs STCOM-1235.
Expand Down
18 changes: 17 additions & 1 deletion lib/CurrencySelect/tests/CurrencySelect-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,26 @@ describe('CurrencySelect', () => {

describe('utility functions', () => {
const CURRENCY_COUNT = 159;
it('expects currency maps to contain the same element counts', () => {
it('expects currency maps to contain the same element counts (reduce by CODE)', () => {
expect(Object.keys(currenciesByCode).length).to.equal(CURRENCY_COUNT);
});

// this test fails with currency-codes v2.2.0 which supplies duplicate
// entries for Bolívar Soberano. it isn't clear if this is intentional
// (and so this map-by-name function should never have been written) or
// accidental (names are unique in previous releases).
//
// if we unpin the dependency from v2.1.0 then we need to remove this function,
// a breaking change. leave comments at STCOM-1379.
it('expects currency maps to contain the same element counts (reduce by NAME)', () => {
expect(Object.keys(currenciesByName).length).to.equal(CURRENCY_COUNT);
});

it('expects currency maps to contain the same element counts (reduce by NUMBER)', () => {
expect(Object.keys(currenciesByNumber).length).to.equal(CURRENCY_COUNT);
});

it('expects currency maps to contain the same element counts', () => {
expect(currenciesOptions.length).to.equal(CURRENCY_COUNT);
});

Expand Down
2 changes: 1 addition & 1 deletion lib/Modal/WrappingElement.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ const WrappingElement = forwardRef(({
focusTrapRef.current.activate();
ATTrap.apply();
}
keyDownHandlerRef.current = listen(document, 'keydown', keyDownHandler);
keyDownHandlerRef.current = listen(modalElementRef.current, 'keydown', keyDownHandler);
focusHandlerRef.current = listen(document, 'focus', enforceFocusTimeout, true);
onOpen();
}
Expand Down
61 changes: 55 additions & 6 deletions lib/RepeatableField/RepeatableField.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, { useRef, useEffect, useState } from 'react';
import classnames from 'classnames';
import PropTypes from 'prop-types';
import { FormattedMessage } from 'react-intl';
Expand All @@ -8,10 +8,9 @@ import Button from '../Button';
import Headline from '../Headline';
import EmptyMessage from '../EmptyMessage';
import IconButton from '../IconButton';
import { RepeatableFieldContent } from "./RepeatableFieldContent";
import { RepeatableFieldContent } from './RepeatableFieldContent';
import css from './RepeatableField.css';
import { useFocusedIndex } from "./hooks/useFocusedIndex";

import { getFirstFocusable } from '../../util/getFocusableElements';

const RepeatableField = ({
canAdd = true,
Expand All @@ -29,15 +28,63 @@ const RepeatableField = ({
onRemove,
renderField,
}) => {
const rootRef = useRef(null);
const showDeleteBtn = typeof onRemove === 'function';
const fieldsLength = fields.length;
const focusedIndex = useFocusedIndex(fieldsLength);
const [hasBeenFocused, setHasBeenFocused] = useState(false);

// use mutation observers to handle focus-management since we only have internal state.
useEffect(() => {
const observer = new MutationObserver(mutations => {
mutations.forEach(mutation => {
if (mutation.type !== 'childList') return;

const addedNode = mutation.addedNodes?.[0];
const removedNode = mutation.removedNodes?.[0];
let rowElem;
// Handle added node
// we check if the user has interacted with the component before handling this, otherwise focus could be
// unwantedly moved when a form is initialized.
if (hasBeenFocused) {
if (addedNode &&
addedNode.nodeType === 1 && // looking for nodeType: element only... not attribute (2) or text (3)
addedNode.matches(`.${css.repeatableFieldItem}`)) { // only apply to repeatable field item addition removal
rowElem = getFirstFocusable(addedNode);
rowElem?.focus();
}
}

// Handle removed node
if (removedNode &&
mutation.previousSibling &&
mutation.previousSibling.matches(`.${css.repeatableFieldItem}`)) {
rowElem = getFirstFocusable(mutation.previousSibling);
rowElem?.focus();
}
});
});

if (rootRef.current) {
// observe for item additions/removals from list.
observer.observe(rootRef.current, {
childList: true,
subtree: true,
});
}

return () => {
observer.disconnect();
};
}, [hasBeenFocused])

return (
<fieldset
ref={rootRef}
id={id}
data-test-repeatable-field
className={classnames(css.repeatableField, { [css.hasMargin]: hasMargin }, className)}
tabIndex="-1"
onFocus={() => { setHasBeenFocused(true) }}
>
{legend && (
<Headline
Expand Down Expand Up @@ -78,7 +125,9 @@ const RepeatableField = ({
key={getFieldUniqueKey(field, index, fields)}
data-test-repeatable-field-list-item
>
<RepeatableFieldContent isFocused={focusedIndex === index} >
<RepeatableFieldContent
rootRef={rootRef}
>
{renderField(field, index, fields)}
</RepeatableFieldContent>
{
Expand Down
31 changes: 8 additions & 23 deletions lib/RepeatableField/RepeatableFieldContent.js
Original file line number Diff line number Diff line change
@@ -1,32 +1,17 @@
import React, { useCallback } from "react";
import PropTypes from "prop-types";
import React from 'react';
import PropTypes from 'prop-types';

import { getFirstFocusable } from "../../util/getFocusableElements";
import css from './RepeatableField.css';

import css from "./RepeatableField.css";

export const RepeatableFieldContent = ({ children, isFocused }) => {
const callbackRef = useCallback((node) => {
if (node) {
const elem = getFirstFocusable(node, true, true);

if (isFocused) {
elem?.focus();
}
}
}, [isFocused])

return (
<div className={css.repeatableFieldItemContent} ref={callbackRef}>
{children}
</div>
);
}
export const RepeatableFieldContent = ({ children }) => (
<div className={css.repeatableFieldItemContent}>
{children}
</div>
);

RepeatableFieldContent.propTypes = {
children: PropTypes.oneOfType([
PropTypes.node,
PropTypes.func,
]).isRequired,
isFocused: PropTypes.bool.isRequired,
}
30 changes: 30 additions & 0 deletions lib/RepeatableField/hooks/useIsElementFocused.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { useEffect, useState } from "react";

export const useIsElementFocused = (ref) => {
const [isFocused, setIsFocused] = useState(false);

useEffect(() => {
const checkIfFocused = () => {
if (ref.current) {
const focusedElement = document.activeElement;
if (ref.current.contains(focusedElement)) {
setIsFocused(true);
} else {
setIsFocused(false);
}
}
};

window.addEventListener("focusin", checkIfFocused);
window.addEventListener("focusout", checkIfFocused);

checkIfFocused();

return () => {
window.removeEventListener("focusin", checkIfFocused);
window.removeEventListener("focusout", checkIfFocused);
};
}, [ref]);

return isFocused;
};
15 changes: 14 additions & 1 deletion lib/Selection/Selection.css
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@


.selectionOuter {
width: 100%;
position: relative;
}

.selectionRoot {
position: relative;
}
Expand All @@ -19,10 +24,18 @@

.selectionList {
list-style: none;
padding: 4px;
padding: 2px 4px;
border-top: 2px solid transparent;
border-bottom: 2px solid transparent;
margin-bottom: 0;
overflow: auto;
position: relative;
outline: 0;

&:focus {
border-top-color: var(--primary);
border-bottom-color: var(--primary);
}
}

.selectListSection {
Expand Down
6 changes: 3 additions & 3 deletions lib/Selection/Selection.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const getControlWidth = (control) => {
const getItemClass = (item, i, props) => {
const { value } = item;
const { selectedItem, highlightedIndex, dataOptions } = props;
if (!value) {
if (value === undefined) {
return;
}

Expand Down Expand Up @@ -260,7 +260,7 @@ const Selection = ({
const rendered = [];
for (let i = 0; i < data.length; i++) {
const item = data[i]
if (item.value) {
if (item.value !== undefined) {
const reducedIndex = reconcileReducedIndex(item, reducedListItems);
rendered.push(
<li
Expand Down Expand Up @@ -424,7 +424,7 @@ const Selection = ({
/* eslint-enable react-hooks/exhaustive-deps */

return (
<div>
<div className={css.selectionOuter}>
{label && (
<Label
{...getLabelProps({
Expand Down
1 change: 1 addition & 0 deletions lib/Selection/SelectionList.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const SelectionList = ({
})}
style={{ maxHeight: listMaxHeight }}
className={css.selectionList}
tabIndex="-1"
>
{ isOpen && renderOptions()}
</ul>
Expand Down
1 change: 1 addition & 0 deletions lib/Selection/stories/BasicUsage.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const hugeOptionsList = syncGenerate(3000, 0, () => {

// the dataOptions prop takes an array of objects with 'label' and 'value' keys
const countriesOptions = [
{ value: '', label: 'blank' },
{ value: 'AU', label: 'Australia' },
{ value: 'CN', label: 'China' },
{ value: 'DK', label: 'Denmark' },
Expand Down
17 changes: 14 additions & 3 deletions lib/Selection/tests/Selection-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ describe('Selection', () => {
{ value: 'test2', label: 'Option 2' },
{ value: 'sample0', label: 'Sample 0' },
{ value: 'invalid', label: 'Sample 1' },
{ value: 'sample2', label: 'Sample 2' }
{ value: 'sample2', label: 'Sample 2' },
{ value: '', label: '' },
];

const groupedOptions = [
Expand Down Expand Up @@ -281,6 +282,16 @@ describe('Selection', () => {
it('focuses the control/trigger', () => {
selection.has({ focused: true });
});

describe('clicking a "blank" option', () => {
beforeEach(async () => {
await selection.choose('');
});

it('sets control value to ""', () => {
selection.has({ value: 'select control' });
});
});
});

describe('filtering options', () => {
Expand Down Expand Up @@ -743,7 +754,7 @@ describe('Selection', () => {
beforeEach(async () => {
filterSpy.resetHistory();
await mountWithContext(
<AsyncFilter filterSpy={filterSpy}/>
<AsyncFilter filterSpy={filterSpy} />
);
});

Expand All @@ -756,7 +767,7 @@ describe('Selection', () => {
await asyncSelection.filterOptions('tes');
});

it ('calls filter function only once (not for each letter)', async () => converge(() => { if (filterSpy.calledTwice) { throw new Error('Selection - onFilter should only be called once.'); }}));
it('calls filter function only once (not for each letter)', async () => converge(() => { if (filterSpy.calledTwice) { throw new Error('Selection - onFilter should only be called once.'); } }));

it('displays spinner in optionsList', () => asyncSelectionList().is({ loading: true }));

Expand Down
Loading

0 comments on commit 5cb1421

Please sign in to comment.