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

Fix layer re-arrangement inside groups using shortcuts #11884

Merged
merged 6 commits into from
Jul 12, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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
31 changes: 16 additions & 15 deletions packages/story-editor/src/app/canvas/useCanvasKeys.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import { __, sprintf } from '@googleforcreators/i18n';
import states from '../highlights/states';
import useHighlights from '../highlights/useHighlights';
import { useStory } from '../story';
import { LAYER_DIRECTIONS } from '../../constants';
import getLayerArrangementProps from './utils/getLayerArrangementProps';
import { useCanvas } from '.';

/**
Expand All @@ -57,6 +57,7 @@ function useCanvasKeys(ref) {
animationState,
updateAnimationState,
currentPageProductIds,
pageElements,
} = useStory(
({
state: {
Expand Down Expand Up @@ -90,6 +91,7 @@ function useCanvasKeys(ref) {
currentPageProductIds: currentPage?.elements
?.filter(({ type }) => type === ELEMENT_TYPES.PRODUCT)
.map(({ product }) => product?.productId),
pageElements: currentPage?.elements,
};
}
);
Expand Down Expand Up @@ -205,12 +207,21 @@ function useCanvasKeys(ref) {
// into mod+left/right triggering the browser's back/forward navigation.
evt.preventDefault();

const layerDir = getLayerDirection(key, shiftKey);
if (layerDir) {
arrangeSelection({ position: layerDir });
// The shortcut doesn't support moving multiple elements currently.
if (selectedElements?.length === 1) {
const { position, groupId } = getLayerArrangementProps(
key,
shiftKey,
selectedElements,
pageElements
);

if (position || groupId) {
arrangeSelection({ position, groupId });
}
}
},
[arrangeSelection]
[arrangeSelection, selectedElements, pageElements]
);

// Edit mode
Expand Down Expand Up @@ -311,14 +322,4 @@ function useCanvasKeys(ref) {
);
}

function getLayerDirection(key, shift) {
if (key === 'ArrowUp') {
return shift ? LAYER_DIRECTIONS.FRONT : LAYER_DIRECTIONS.FORWARD;
}
if (key === 'ArrowDown') {
return shift ? LAYER_DIRECTIONS.BACK : LAYER_DIRECTIONS.BACKWARD;
}
return null;
}

export default useCanvasKeys;
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
* Copyright 2022 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* Internal dependencies
*/
import { LAYER_DIRECTIONS } from '../../../constants';

function getLayerArrangementProps(key, shift, selectedElements, elements) {
// This only supports moving single layer.
if (!selectedElements || selectedElements.length > 1) {
return {};
}
let position = null;
if (key === 'ArrowUp') {
position = shift ? LAYER_DIRECTIONS.FRONT : LAYER_DIRECTIONS.FORWARD;
}
if (key === 'ArrowDown') {
position = shift ? LAYER_DIRECTIONS.BACK : LAYER_DIRECTIONS.BACKWARD;
}
if (!position) {
return {};
}

const { id, groupId } = selectedElements[0];
// Get layer index.
const currentPosition = elements.findIndex(({ id: elId }) => elId === id);

// If the layer is in a group, check if it's first or last of the group,
// in this case we should just move it out of the group.
if (groupId) {
const isLastInGroup = elements[currentPosition - 1]?.groupId !== groupId;
const isFirstInGroup = elements[currentPosition + 1]?.groupId !== groupId;
if (
(isLastInGroup && position === LAYER_DIRECTIONS.BACKWARD) ||
(isFirstInGroup && position === LAYER_DIRECTIONS.FORWARD)
) {
return {
position: currentPosition,
groupId: null,
};
}
barklund marked this conversation as resolved.
Show resolved Hide resolved
} else {
// If the element has a group below, just add it to the group.
if (
elements[currentPosition - 1]?.groupId &&
position === LAYER_DIRECTIONS.BACKWARD
) {
return {
position: currentPosition,
groupId: elements[currentPosition - 1].groupId,
};
}
// If the element has a group above, just add it to the group.
if (
elements[currentPosition + 1]?.groupId &&
position === LAYER_DIRECTIONS.FORWARD
) {
return {
position: currentPosition,
groupId: elements[currentPosition + 1].groupId,
};
}
}

// Otherwise let's just change the position.
return { position };
}

export default getLayerArrangementProps;
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
/*
* Copyright 2022 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* Internal dependencies
*/
import getLayerArrangementProps from '../getLayerArrangementProps';
import { LAYER_DIRECTIONS } from '../../../../constants';

describe('getLayerArrangementProps', () => {
it('should do nothing when more or less than one element is selected', () => {
expect(
getLayerArrangementProps(null, null, [{ id: '1' }, { id: '2' }])
).toStrictEqual({});
expect(getLayerArrangementProps(null, null, [])).toStrictEqual({});
});

it('should do nothing if irrelevant key is pressed', () => {
expect(
getLayerArrangementProps('ArrowLeft', null, [{ id: '1' }])
).toStrictEqual({});
});

it('should get the layer position as expected within layers without groups', () => {
const elements = [
{ id: 'a', isBackground: true },
{ id: 'b' },
{ id: 'c' },
{ id: 'd' },
];
expect(
getLayerArrangementProps('ArrowUp', false, [{ id: 'b' }], elements)
).toStrictEqual({ position: LAYER_DIRECTIONS.FORWARD });
expect(
getLayerArrangementProps('ArrowDown', false, [{ id: 'c' }], elements)
).toStrictEqual({ position: LAYER_DIRECTIONS.BACKWARD });
});

it('should get the layer group and position correctly for first/last layers of the group', () => {
const elements = [
{ id: 'a', isBackground: true },
{ id: 'b' },
{ id: 'c', groupId: 'g1' },
{ id: 'd', groupId: 'g1' },
{ id: 'e', groupId: 'g2' },
];
// Position stays the same but the layer is moved out of the group.
expect(
getLayerArrangementProps(
'ArrowUp',
false,
[{ id: 'd', groupId: 'g1' }],
elements
)
).toStrictEqual({ position: 3, groupId: null });
expect(
getLayerArrangementProps(
'ArrowDown',
false,
[{ id: 'c', groupId: 'g1' }],
elements
)
).toStrictEqual({ position: 2, groupId: null });
});

it('should get the layer group and position correctly when moving an element inside a group', () => {
const elements = [
{ id: 'a', isBackground: true },
{ id: 'b' },
{ id: 'c', groupId: 'g1' },
{ id: 'd', groupId: 'g1' },
{ id: 'e', groupId: 'g1' },
{ id: 'f' },
];
expect(
getLayerArrangementProps(
'ArrowUp',
false,
[{ id: 'd', groupId: 'g1' }],
elements
)
).toStrictEqual({ position: LAYER_DIRECTIONS.FORWARD });
expect(
getLayerArrangementProps(
'ArrowDown',
false,
[{ id: 'd', groupId: 'g1' }],
elements
)
).toStrictEqual({ position: LAYER_DIRECTIONS.BACKWARD });
});

it('should get the layer group and position correctly when moving an element into a group', () => {
const elements = [
{ id: 'a', isBackground: true },
{ id: 'b' },
{ id: 'c', groupId: 'g1' },
{ id: 'd' },
{ id: 'e', groupId: 'g2' },
];
// Position stays the same but the layer is moved out of the group.
miina marked this conversation as resolved.
Show resolved Hide resolved
expect(
getLayerArrangementProps('ArrowUp', false, [{ id: 'd' }], elements)
).toStrictEqual({ position: 3, groupId: 'g2' });
expect(
getLayerArrangementProps('ArrowDown', false, [{ id: 'd' }], elements)
).toStrictEqual({ position: 3, groupId: 'g1' });
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,10 @@ const arrangeGroup =

const arrangeSelection =
(dispatch) =>
({ position }) =>
({ position, groupId }) =>
dispatch({
type: types.ARRANGE_ELEMENT,
payload: { elementId: null, position },
payload: { elementId: null, position, groupId },
});

const setSelectedElementsById =
Expand Down