-
Notifications
You must be signed in to change notification settings - Fork 321
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
Add support for multi-column layouts #37
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
b45f1d2
to
3a41603
Compare
03369f1
to
bb7c88b
Compare
Can't wait for this to be merged |
Me neither @ahmedrowaihi-official! It's going to be a bit of a game-changer. Regarding help - any feedback on the API would be great, if you have any? Otherwise I just need some time to push it over the line. I hope by the end of this coming week! |
On this particular PR no, but I might do some refactoring as for imports/exports consistency as well as fixing packages resolutions for the whole puck packages |
@ahmedrowaihi-official Sure! I'd consider waiting until this PR lands as it will touch a lot of that stuff. |
Really nice UX |
40b68c8
to
de15df0
Compare
ce774a9
to
cbe0b02
Compare
d1419c0
to
d0f5776
Compare
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.
UI QA and comments
- Demo outputs
register zone
messages to console, intentional? - Expected the chevron here to indicate the item could be show/hide toggled. Wasn't immediately clear to me that ButtonGroup was a child of Flex (in fact, maybe it isn't?)
- 🦊 Firefox
- No parent > child highlighting, expected due to lack of
:has
support 👍🏻 0.75
zoom effect on the preview doesn't work, expected sincezoom
is non-standard & not supported in FF- These two in combination do mean the FF UI is noticeably not as nice as in Chrome/Safari, especially on my small screen. But
:has
is coming 🙏🏻
- No parent > child highlighting, expected due to lack of
- 🧱 Demo components
- Be good to have a way of making these Cards full height
- Column spanning also doesn't do anything unless
distribution="Manual"
selected, which confused me for a minute. - I can get the Columns span selector in a weird state. In general I avoid
type="number"
cause of stuff like this - Also Flex minimum item width
zoom: 1.33; | ||
position: relative; | ||
height: 100%; | ||
outline-offset: -1; |
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.
Missing a px
One more thing from me
|
@chrisvxd happy for all these to be spun out, certainly not blocking on this PR 👍🏻 |
@monospaced Tracking all remaining tasks in #100. |
💯 |
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.
🎤 drop
[Updated Reply]: |
Coming in late here, user of rbd - wondering how you managed to get the onMouseOver/onMouseOut events to fire whilst a drag was active. |
found it, |
Ah nice one @larowlan - was meaning to reply and say |
Thanks for following up ❤️ |
Description
This PR adds:
<DropZone />
component.Unfortunately this results in a rather large PR that was hard to envision before starting the work. Further work should be smaller following this significant effort.
Closes #43
Example
Implementation
This PR adds a new
<DropZone>
component that replaces the corereact-beautiful-dnd
droppable functionality that was previously kept in the<Puck>
component, and exposes this to the user. It's essentially a wrapper around react-beautiful-dnd, with additional logic to support infinitely nested DropZones, application-level context and a robust UX.Specifically, the
<DropZone>
component:The Rules of DropZones
Data model
To support this, we add the
dropzones
key to the Data model. This is an additive, non-breaking change.object
)object[]
) : Same shape as thecontent
field on DatadropzoneCompoundId is a string that takes the shape
area:dropzone
.area
is the id of the parent component that contains this dropzone.dropzone
is the ID provided by the user when using the DropZone.Rendering multiple columns
Example data model for a page containing a Columns component at the root, which renders 2 columns. The first column contains a Heading component.
This config:
Produces this data:
Retaining backwards compatibility with the root DropZone
In order to retain backwards compatibility and avoid a breaking change, I made the decision to keep the existing
content
key on the root ofData
when the user is not using the DropZone component.Under the hood, the default drop zone (identified as
puck-drop-zone
) is no different to any other drop zone. It just stores its data undercontent
, and there is some additional logic (mostly in the newly introduced reducer) to handle the relevant data operations.Rendering multiple root DropZones
If you render
children
in yourroot
component, you'll render the default puck drop zone (puck-drop-zone
). However, you can also choose to create multiple root drop zones.Results in the data
Note that, in this example, content is empty since we're not rendering the default dropzone via
children
.Questions
id
,dropzone
ordropZone
for the DropZone identity prop?*root
andcontent
fields in Data for the default dropzone, or merge the default DropZone behaviour into thedropzones
key?*dropzones
toareas
?*{"areaId:dropzoneId": []}
to nested objects like{"areaId": {"dropzoneId: []}}"
"? This may be cleaner from a data model point of view, but will require some rearchitecture.*puck-drop-zone
todefault
?*Breaking changes
Tasks
dropzone
prop todropZone
orid
Future tasks
Further examples
Dynamic columns