-
Notifications
You must be signed in to change notification settings - Fork 178
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
Background element refactor (+ drop targets and fixes) #693
Changes from all commits
caf08c0
6496812
7b28950
522d0e4
2fe69f7
7fa5a16
5bc4cc4
c2348e7
c99b3b8
67efbbb
07847bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
/* | ||
* Copyright 2020 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. | ||
*/ | ||
|
||
/** | ||
* External dependencies | ||
*/ | ||
import { useEffect } from 'react'; | ||
/** | ||
* Internal dependencies | ||
*/ | ||
import { createNewElement } from '../../../elements'; | ||
import theme from '../../../theme'; | ||
import { PAGE_WIDTH } from '../../../constants'; | ||
import { MaskTypes } from '../../../masks'; | ||
import createSolidFromString from '../../../utils/createSolidFromString'; | ||
|
||
// By default, the element should be 33% of the page. | ||
const DEFAULT_ELEMENT_WIDTH = PAGE_WIDTH / 3; | ||
|
||
// Make sure pages have background elements at all times | ||
function usePageBackgrounds({ currentPage, setBackgroundElement, addElement }) { | ||
useEffect(() => { | ||
if (!currentPage || currentPage?.backgroundElementId) { | ||
return; | ||
} | ||
const element = createNewElement('shape', { | ||
x: (PAGE_WIDTH / 4) * Math.random(), | ||
y: (PAGE_WIDTH / 4) * Math.random(), | ||
width: DEFAULT_ELEMENT_WIDTH, | ||
height: DEFAULT_ELEMENT_WIDTH, | ||
rotationAngle: 0, | ||
mask: { | ||
type: MaskTypes.RECTANGLE, | ||
}, | ||
flip: { | ||
vertical: false, | ||
horizontal: false, | ||
}, | ||
isBackground: true, | ||
backgroundColor: createSolidFromString(theme.colors.fg.v1), | ||
}); | ||
addElement({ element }); | ||
setBackgroundElement({ elementId: element.id }); | ||
}, [addElement, currentPage, setBackgroundElement]); | ||
} | ||
|
||
export default usePageBackgrounds; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,7 +63,7 @@ function FrameElement({ element }) { | |
actions: { setNodeForElement, handleSelectElement }, | ||
} = useCanvas(); | ||
const { | ||
state: { selectedElementIds }, | ||
state: { selectedElementIds, currentPage }, | ||
} = useStory(); | ||
const { | ||
actions: { getBox }, | ||
|
@@ -77,6 +77,7 @@ function FrameElement({ element }) { | |
}, [id, setNodeForElement]); | ||
const isSelected = selectedElementIds.includes(id); | ||
const box = getBox(element); | ||
const isBackground = currentPage?.backgroundElementId === id; | ||
|
||
return ( | ||
<Wrapper | ||
|
@@ -87,7 +88,9 @@ function FrameElement({ element }) { | |
if (!isSelected) { | ||
handleSelectElement(id, evt); | ||
} | ||
evt.stopPropagation(); | ||
if (!isBackground) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this? Could you add a comment in code. This seems like an important behavior involving background elements. As such, I'm also wondering if it'd be safer to roll it all inside the |
||
evt.stopPropagation(); | ||
} | ||
}} | ||
onFocus={(evt) => { | ||
if (!isSelected) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,6 +57,7 @@ const Lasso = styled.div` | |
function SelectionCanvas({ children }) { | ||
const { | ||
actions: { clearSelection }, | ||
state: { selectedElements }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we use |
||
} = useStory(); | ||
const { | ||
state: { pageContainer }, | ||
|
@@ -98,8 +99,10 @@ function SelectionCanvas({ children }) { | |
}; | ||
|
||
const onMouseDown = (evt) => { | ||
clearSelection(); | ||
clearEditing(); | ||
if (selectedElements.length) { | ||
clearSelection(); | ||
clearEditing(); | ||
} | ||
|
||
const overlay = overlayRef.current; | ||
let offsetX = 0, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we update the model itself, I think it'd work better in the story reducer rather than a disconnected effect. For one, we wouldn't have history problems with this approach. This might be an important thing to fix right away.