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

proof of concept at getting snap to grid #16748

Closed
wants to merge 6 commits into from

Conversation

senadir
Copy link
Contributor

@senadir senadir commented Jul 25, 2019

This is an early stage draft PR to see how can we implement snap to grid into Gutenberg.

I will follow this PR with a RFC later that have all the technical details of how will snapping work, possible behaviors and problems.

for now this PR only adds:

  • simple snapping to the Image block
  • it define a fixed grid with fixed stops
  • only support left align snapping
  • doesn't support full width snapping

the main things I thing we should consider is defining:

  • how should we calculate the grid columns & stops
  • how should the technical integration & data sharing be (for now I'm using core/block-editor with a couple of actions/selectors to see if the grid should be visible or not)

future things we should consider are:

  • how can theme authors define a custom grid
  • how should we handle things like responsiveness
  • accessibility, both keyboard & screen reader

this is a live prototype I made that doesn't use Gutenberg components, vanilla react, it should be a good basis to see how does snapping work
https://gutenberg-snapping.netlify.com/

and this is a gif of how the snapping work in Gutenberg
snapping

@senadir senadir added [Block] Image Affects the Image Block [Package] Block editor /packages/block-editor [Type] Feedback Issues that relate purely to feedback on a feature that isn't necessarily actionable Needs Design Feedback Needs general design feedback. Needs Technical Feedback Needs testing from a developer perspective. [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible labels Jul 25, 2019
@mapk
Copy link
Contributor

mapk commented Jul 31, 2019

Related to #16271 There are questions in the issue that should also be discussed further in relation to this PR. It's looking wonderful!

@senadir
Copy link
Contributor Author

senadir commented Jul 31, 2019

@mapk I'm surprised that this is the first time I'm seeing that issue 😅 I actually prepared (not finished) a RFC that addresses most of those questions & possible approaches to them, I will share it as soon as I finish it and will contribute to that issue

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you think this fits with the general idea of adding specific "grid lines" for the wide and full alignments?

How can we make it possible to resize to the "wide" grid line and automatically switch the block alignment to "wide" in that case?...

{ 'block-editor-block-list__layout--grid-visible': isGridVisible }
) }
>
{ this.generateGridLines() }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be a separate component?


// snap gap is sued to snap the image to the line when it sizes approach this point
gap: 20,
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be the perfect API here? I feel the "gap" shouldn't be part of the API and just an implementation detail. and maybe instead of gridStops, we'd just provide the number of columns in the main area?

I also think the same config should be possible to override in InnerBlocks I feel each inner block area could potentially define its own "grid".

slug: 'vivid-red',
color: '#cf2e2e',
},
{ name: __( 'Vivid red' ), slug: 'vivid-red', color: '#cf2e2e' },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess automatic code style change. Should we revert?

@@ -879,6 +880,7 @@ export class ImageEdit extends Component {
left: showLeftHandle,
} }
onResizeStart={ () => {
this.props.showGrid();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think several blocks could make use of ResizableBox and the grid config together should we create a component that is already bound to reuse between the blocks that make use of it GridResizableBox or something?

@draganescu
Copy link
Contributor

I wanted to test this but the conflicts kept me from doing so. @senadir do you have some time to rebase please? 😊🙌👏

@senadir
Copy link
Contributor Author

senadir commented Nov 19, 2019

I wanted to test this but the conflicts kept me from doing so. @senadir do you have some time to rebase, please? 😊🙌👏

Absolutely I should find sometime later today or tomorrow to rebase and possibly rewrite this proof of concept to a more modern method

@senadir senadir requested a review from ajlende as a code owner October 1, 2020 09:40
@paaljoachim
Copy link
Contributor

I see this PR has stopped up.
Can we get a status update?
What can we do to move this forward?

@glendaviesnz Glen I am pinging you in here as I am thinking about the work your doing on the Gallery, and think that this PR might be something to explore later on.

@youknowriad
Copy link
Contributor

This PR was a nice experiment but I think if we want to introduce this now, we'd do differently using the "layout" prop for innerBlocks, so it's basically an entire rewrite. I'm going to just close it for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Image Affects the Image Block Needs Design Feedback Needs general design feedback. Needs Technical Feedback Needs testing from a developer perspective. [Package] Block editor /packages/block-editor [Type] Feedback Issues that relate purely to feedback on a feature that isn't necessarily actionable [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants