Skip to content

Commit

Permalink
Merge branch 'master' into beta
Browse files Browse the repository at this point in the history 
* master:
  MBS-12937: Changing credits should work without modifying the source relationship (#2865)
  MBS-12936: Don't update Autocomplete2 scroll position on item mouseover (#2864)
  • Loading branch information
@mwiencek
mwiencek committed Feb 24, 2023
2 parents bfc9217 + 9fcb286 commit 71719b3
Show file tree
Hide file tree
Showing 5 changed files with 201 additions and 19 deletions.
19 changes: 7 additions & 12 deletions root/static/scripts/common/components/Autocomplete2.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ const Autocomplete2 = (React.memo(<+T: EntityItemT>(
const inputTimeout = React.useRef<TimeoutID | null>(null);
const containerRef = React.useRef<HTMLDivElement | null>(null);
const prevIsOpen = React.useRef<boolean>(false);
const prevHighlightedIndex = React.useRef<number>(-1);
const shouldUpdateScrollPositionRef = React.useRef<boolean>(false);

const highlightedItem = highlightedIndex >= 0
? (items[highlightedIndex] ?? null)
Expand Down Expand Up @@ -528,6 +528,7 @@ const Autocomplete2 = (React.memo(<+T: EntityItemT>(
event.preventDefault();

if (isOpen) {
shouldUpdateScrollPositionRef.current = true;
dispatch(HIGHLIGHT_NEXT_ITEM);
} else {
showAvailableItemsOrBeginLookupOrSearch();
Expand All @@ -537,6 +538,7 @@ const Autocomplete2 = (React.memo(<+T: EntityItemT>(
case 'ArrowUp':
if (isOpen) {
event.preventDefault();
shouldUpdateScrollPositionRef.current = true;
dispatch(HIGHLIGHT_PREVIOUS_ITEM);
}
break;
Expand Down Expand Up @@ -676,22 +678,15 @@ const Autocomplete2 = (React.memo(<+T: EntityItemT>(

React.useLayoutEffect(() => {
const shouldUpdateScrollPosition = (
isOpen &&
(
!prevIsOpen.current ||
highlightedIndex !== prevHighlightedIndex.current
)
(isOpen && !prevIsOpen.current) ||
shouldUpdateScrollPositionRef.current
);
prevIsOpen.current = isOpen;
prevHighlightedIndex.current = highlightedIndex;
if (shouldUpdateScrollPosition) {
setScrollPosition(menuId);
shouldUpdateScrollPositionRef.current = false;
}
}, [
isOpen,
highlightedIndex,
menuId,
]);
});

type AutocompleteItemComponent<T> =
React$AbstractComponent<AutocompleteItemPropsT<T>, void>;
Expand Down
32 changes: 25 additions & 7 deletions root/static/scripts/relationship-editor/components/RelationshipEditor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -435,17 +435,30 @@ export function runReducer(
sourceEntity,
} = action;

const relationshipStateChanged = (
oldRelationshipState != null &&
!relationshipsAreIdentical(
oldRelationshipState,
newRelationshipState,
)
);

if (
oldRelationshipState == null ||
!relationshipsAreIdentical(oldRelationshipState, newRelationshipState)
relationshipStateChanged ||
creditsToChangeForSource ||
creditsToChangeForTarget
) {
const targetEntity = getRelationshipTarget(
newRelationshipState,
sourceEntity,
);
const updates = [];

if (oldRelationshipState != null) {
if (
oldRelationshipState != null &&
relationshipStateChanged
) {
/*
* The old relationship state must be removed first in a separate
* `updateRelationships` call, because its presence affects other
Expand All @@ -464,11 +477,16 @@ export function runReducer(
);
}

updates.push(...getUpdatesForAcceptedRelationship(
writableState,
newRelationshipState,
sourceEntity,
));
if (
oldRelationshipState == null ||
relationshipStateChanged
) {
updates.push(...getUpdatesForAcceptedRelationship(
writableState,
newRelationshipState,
sourceEntity,
));
}

/*
* `updateEntityCredits` only uses `newRelationshipState` to obtain
Expand Down
65 changes: 65 additions & 0 deletions root/static/scripts/tests/relationship-editor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,71 @@ test('splitRelationshipByAttributes', function (t) {
);
});

test('MBS-12937: Changing credits for other relationships without modifying the source relationship', function (t) {
t.plan(1);

const relationship1 = {
_lineage: [],
_original: null,
_status: REL_STATUS_ADD,
attributes: null,
begin_date: null,
editsPending: false,
end_date: null,
ended: false,
entity0: artist,
entity0_credit: 'SOMECREDIT',
entity1: recording,
entity1_credit: '',
id: -1,
linkOrder: 0,
linkTypeID: 148,
};

const relationship2 = {
...relationship1,
entity0_credit: '',
id: -2,
linkTypeID: 297,
};

Object.freeze(relationship1);
Object.freeze(relationship2);

let state = addRelationships(
initialState,
recording,
[relationship1, relationship2],
);

state = reducer(
state,
{
batchSelectionCount: 0,
creditsToChangeForSource: '',
creditsToChangeForTarget: 'all',
newRelationshipState: relationship1,
oldRelationshipState: relationship1,
sourceEntity: recording,
type: 'update-relationship-state',
},
);

currentRelationshipsEqual(
t,
state,
[
relationship1,
{
...relationship2,
entity0_credit: 'SOMECREDIT',
},
],
'all entity credits are updated despite not modifying the source ' +
'relationship',
);
});

function addRelationships(
rootState: RelationshipEditorStateT,
source: CoreEntityT,
Expand Down
1 change: 1 addition & 0 deletions t/selenium.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ const KEY_CODES = {
'${KEY_HOME}': Key.HOME,
'${KEY_SHIFT}': Key.SHIFT,
'${KEY_TAB}': Key.TAB,
'${KEY_UP}': Key.ARROW_UP,
'${MBS_ROOT}': DBDefs.MB_SERVER_ROOT.replace(/\/$/, ''),
};

Expand Down
103 changes: 103 additions & 0 deletions t/selenium/Autocomplete2.json5
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,108 @@
value: '0',
},
// End of test for MBS-12631.
// MBS-12936: The menu scroll position should only be updated when
// navigating items using the arrow keys, or when initially opening the
// menu (to reset it to the top). Hovering over items with the mouse
// should explicitly not affect the scroll position.
// The menu should now be open; first check that the scroll position is
// initially zero:
{
command: 'assertEval',
target: 'document.querySelector("#attribute-type-test-menu").scrollTop',
value: '0',
},
// Simulate a mouseover event on the last item.
{
command: 'runScript',
target: '\
document.querySelector("#attribute-type-test-menu li:last-child")\
.dispatchEvent(new Event("mouseover", {bubbles: true}))\
',
value: '',
},
// Make sure the last item is highlighted.
{
command: 'assertEval',
target: 'document.querySelector("#attribute-type-test-menu li:last-child[aria-selected=true]") != null',
value: 'true',
},
// Check that the scroll position did not change.
{
command: 'assertEval',
target: 'document.querySelector("#attribute-type-test-menu").scrollTop',
value: '0',
},
{
command: 'type',
target: 'id=attribute-type-test-input',
value: '',
},
// Close the menu, open it with the down arrow key, and then move to the
// last item with the up arrow key.
{
command: 'pause',
target: '100',
value: '',
},
{
command: 'sendKeys',
target: 'id=attribute-type-test-input',
value: '${KEY_ESC}',
},
{
command: 'pause',
target: '100',
value: '',
},
{
command: 'sendKeys',
target: 'id=attribute-type-test-input',
value: '${KEY_DOWN}',
},
{
command: 'pause',
target: '100',
value: '',
},
{
command: 'sendKeys',
target: 'id=attribute-type-test-input',
value: '${KEY_UP}',
},
// Check that the scroll position changed.
{
command: 'assertEval',
target: 'document.querySelector("#attribute-type-test-menu").scrollTop > 0',
value: 'true',
},
// Close the menu, open it with the down arrow key, and check that the
// scroll position is back at the top.
{
command: 'pause',
target: '100',
value: '',
},
{
command: 'sendKeys',
target: 'id=attribute-type-test-input',
value: '${KEY_ESC}',
},
{
command: 'pause',
target: '100',
value: '',
},
{
command: 'sendKeys',
target: 'id=attribute-type-test-input',
value: '${KEY_DOWN}',
},
{
command: 'assertEval',
target: 'document.querySelector("#attribute-type-test-menu").scrollTop',
value: '0',
},
// End of test for MBS-12936.
],
}

0 comments on commit 71719b3

Please sign in to comment.