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

Completely disallow percentage based group children. #4302

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export type GroupValidity = 'valid' | 'warning' | 'error'

export type InvalidGroupState =
| 'child-not-position-absolute'
| 'child-has-percentage-pins-without-group-size'
| 'child-has-percentage-pins'
| 'child-has-missing-pins'
| 'child-does-not-honour-props-position'
| 'child-does-not-honour-props-size'
Expand All @@ -103,7 +103,7 @@ export type InvalidGroupState =

export function groupValidityFromInvalidGroupState(groupState: InvalidGroupState): GroupValidity {
switch (groupState) {
case 'child-has-percentage-pins-without-group-size':
case 'child-has-percentage-pins':
case 'child-has-missing-pins':
case 'group-has-percentage-pins':
return 'warning'
Expand Down Expand Up @@ -139,8 +139,8 @@ export function invalidGroupStateToString(state: InvalidGroupState): string {
// children state
case 'child-not-position-absolute':
return 'All children of group should have absolute position'
case 'child-has-percentage-pins-without-group-size':
return 'Group needs dimensions to use children with % pins'
case 'child-has-percentage-pins':
return 'Children of a group should not have percentage pins'
case 'child-has-missing-pins':
return 'All children of group should have valid pins'
case 'child-does-not-honour-props-position':
Expand Down Expand Up @@ -261,7 +261,6 @@ function maybeInvalidGroupChildren(
allElementProps: AllElementProps,
projectContents: ProjectContentTreeRoot,
): InvalidGroupState | 'valid' {
const groupHasExplicitSize = checkGroupHasExplicitSize(group)
const childPaths = MetadataUtils.getChildrenUnordered(metadata, path).map(
(child) => child.elementPath,
)
Expand All @@ -274,11 +273,7 @@ function maybeInvalidGroupChildren(
for (const childPath of flattenedChildPaths) {
const childMetadata = MetadataUtils.findElementByElementPath(metadata, childPath)
if (childMetadata != null) {
const childGroupState = getGroupChildState(
projectContents,
childMetadata,
groupHasExplicitSize,
)
const childGroupState = getGroupChildState(projectContents, childMetadata)
if (isInvalidGroupState(childGroupState)) {
return childGroupState
}
Expand All @@ -287,13 +282,10 @@ function maybeInvalidGroupChildren(
return 'valid'
}

export function getGroupChildStateFromJSXElement(
jsxElement: JSXElement,
groupHasExplicitSize: boolean,
): GroupState {
export function getGroupChildStateFromJSXElement(jsxElement: JSXElement): GroupState {
return (
maybeGroupChildNotPositionAbsolutely(jsxElement) ??
maybeGroupChildHasPercentagePinsWithoutGroupSize(jsxElement, groupHasExplicitSize) ??
maybeGroupChildHasPercentagePinsWithoutGroupSize(jsxElement) ??
maybeGroupChildHasMissingPins(jsxElement) ??
'valid'
)
Expand All @@ -313,7 +305,6 @@ export function getGroupValidity(
function getGroupChildState(
projectContents: ProjectContentTreeRoot,
elementMetadata: ElementInstanceMetadata | null,
groupHasExplicitSize: boolean,
): GroupState {
if (elementMetadata == null) {
return 'unknown'
Expand All @@ -332,7 +323,7 @@ function getGroupChildState(
return 'child-does-not-honour-props-size'
}

return getGroupChildStateFromJSXElement(jsxElement, groupHasExplicitSize)
return getGroupChildStateFromJSXElement(jsxElement)
}

export function getGroupChildStateWithGroupMetadata(
Expand All @@ -345,11 +336,7 @@ export function getGroupChildStateWithGroupMetadata(
return 'unknown'
}

return getGroupChildState(
projectContents,
elementMetadata,
checkGroupHasExplicitSize(groupElement),
)
return getGroupChildState(projectContents, elementMetadata)
}

export function isMaybeGroupForWrapping(element: JSXElementChild, imports: Imports): boolean {
Expand Down Expand Up @@ -415,20 +402,11 @@ export function maybeGroupChildNotPositionAbsolutely(

export function maybeGroupChildHasPercentagePinsWithoutGroupSize(
jsxElement: JSXElement | null,
groupHasExplicitSize: boolean,
): InvalidGroupState | null {
if (jsxElement == null) {
return 'unknown'
}
return !groupHasExplicitSize && elementHasPercentagePins(jsxElement)
? 'child-has-percentage-pins-without-group-size'
: null
}

export function maybeGroupChildWithoutFixedSizeForFill(
group: JSXElement | null,
): InvalidGroupState | null {
return !checkGroupHasExplicitSize(group) ? 'child-has-percentage-pins-without-group-size' : null
return elementHasPercentagePins(jsxElement) ? 'child-has-percentage-pins' : null
}

export function groupErrorToastCommand(state: InvalidGroupState): ShowToastCommand {
Expand Down Expand Up @@ -500,8 +478,7 @@ export function groupStateFromJSXElement(
)
} else if (treatElementAsGroupLike(metadata, EP.parentPath(path))) {
// group child
const group = MetadataUtils.getJSXElementFromMetadata(metadata, EP.parentPath(path))
return getGroupChildStateFromJSXElement(element, checkGroupHasExplicitSize(group))
return getGroupChildStateFromJSXElement(element)
} else {
// not a group
return null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3379,7 +3379,61 @@ describe('inspector tests with real metadata', () => {
await setControlValue('position-left-number-input', '25%', renderResult.renderedDOM)

expect(getFrame(targetPath, renderResult)).toBe(elementFrame)
expectGroupToast(renderResult, 'child-has-percentage-pins-without-group-size')
expectGroupToast(renderResult, 'child-has-percentage-pins')
})
it('ignores settings percentage pins on a group child if the parent has explicit width and height', async () => {
const renderResult = await renderTestEditorWithCode(
makeTestProjectCodeWithSnippetStyledComponents(`
<div
style={{ position: 'absolute', backgroundColor: '#FFFFFF' }}
data-uid='aaa'
>
<Group
data-uid='group'
style={{
position: 'absolute',
left: 10,
top: 10,
width: 100,
height: 100,
}}
>
<div
data-uid='foo'
style={{
position: 'absolute',
top: 10,
bottom: 20,
left: 30,
right: 40,
}}
/>
</Group>
</div>
`),
'await-first-dom-report',
)

const targetPath = EP.appendNewElementPath(TestScenePath, ['aaa', 'group', 'foo'])

await act(async () => {
const dispatchDone = renderResult.getDispatchFollowUpActionsFinished()
await renderResult.dispatch([selectComponents([targetPath], false)], false)
await dispatchDone
})

const leftControl = (await renderResult.renderedDOM.findByTestId(
'position-left-number-input',
)) as HTMLInputElement

expect(leftControl.value).toBe('30')

const elementFrame = getFrame(targetPath, renderResult)

await setControlValue('position-left-number-input', '25%', renderResult.renderedDOM)

expect(getFrame(targetPath, renderResult)).toBe(elementFrame)
expectGroupToast(renderResult, 'child-has-percentage-pins')
})
describe('group children', () => {
const tests: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ import { getFramePointsFromMetadata, MaxContent } from '../inspector-common'
import { mapDropNulls } from '../../../core/shared/array-utils'
import {
maybeInvalidGroupState,
maybeGroupChildWithoutFixedSizeForFill,
groupErrorToastAction,
} from '../../canvas/canvas-strategies/strategies/group-helpers'

Expand Down Expand Up @@ -416,11 +415,7 @@ export function usePinToggling(): UsePinTogglingResult {
: null
},
onGroupChild: (path) => {
const group = MetadataUtils.getJSXElementFromMetadata(
jsxMetadataRef.current,
EP.parentPath(path),
)
return maybeGroupChildWithoutFixedSizeForFill(group) ?? null
return 'child-has-percentage-pins'
},
},
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
} from '../../canvas/commands/set-css-length-command'
import {
groupErrorToastCommand,
maybeGroupChildWithoutFixedSizeForFill,
maybeInvalidGroupState,
} from '../../canvas/canvas-strategies/strategies/group-helpers'

Expand All @@ -45,8 +44,7 @@ export const fillContainerStrategyFlow = (
const invalidGroupState = maybeInvalidGroupState(elements, metadata, {
onGroup: () => 'group-has-percentage-pins',
onGroupChild: (path) => {
const group = MetadataUtils.getJSXElementFromMetadata(metadata, EP.parentPath(path))
return maybeGroupChildWithoutFixedSizeForFill(group) ?? null
return 'child-has-percentage-pins'
},
})
if (invalidGroupState != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import type { InspectorStrategy } from './inspector-strategy'
import { queueGroupTrueUp } from '../../canvas/commands/queue-group-true-up-command'
import {
groupErrorToastCommand,
maybeGroupChildWithoutFixedSizeForFill,
maybeInvalidGroupState,
} from '../../canvas/canvas-strategies/strategies/group-helpers'
import { trueUpElementChanged } from '../../../components/editor/store/editor-state'
Expand All @@ -32,8 +31,7 @@ export const fixedSizeBasicStrategy = (
const invalidGroupState = maybeInvalidGroupState(elementPaths, metadata, {
onGroup: () => (value.unit === '%' ? 'group-has-percentage-pins' : null),
onGroupChild: (path) => {
const group = MetadataUtils.getJSXElementFromMetadata(metadata, EP.parentPath(path))
return value.unit === '%' ? maybeGroupChildWithoutFixedSizeForFill(group) : null
return value.unit === '%' ? 'child-has-percentage-pins' : null
},
})
if (invalidGroupState != null) {
Expand Down
Loading