Skip to content

Implement forms row layout #553

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

Merged
merged 11 commits into from
Mar 28, 2023
Merged

Implement forms row layout #553

merged 11 commits into from
Mar 28, 2023

Conversation

pinussilvestrus
Copy link
Contributor

@pinussilvestrus pinussilvestrus commented Feb 21, 2023

Closes #560
Depends on bpmn-io/properties-panel#230

Check out the demo here.


This includes

  • Add Carbon grid under the hood
  • Support columns in the viewer
  • Refactor dragging into module
  • Support columns in the editor
  • Validating rows restrictions
    • prevent overflows: maximum 16 columns per row
    • maximum 4 fields per row

(Potential) follow ups

image

@bpmn-io-tasks bpmn-io-tasks bot added the in progress Currently worked on label Feb 21, 2023
@github-actions github-actions bot temporarily deployed to demo-spike-forms-row-layout February 21, 2023 10:48 Destroyed
@github-actions github-actions bot temporarily deployed to demo-spike-forms-row-layout February 21, 2023 10:48 Destroyed
@github-actions github-actions bot temporarily deployed to demo-spike-forms-row-layout February 21, 2023 15:35 Destroyed
@pinussilvestrus pinussilvestrus force-pushed the spike-forms-row-layout branch from 68fa1d3 to 852f582 Compare March 3, 2023 12:54
@github-actions github-actions bot temporarily deployed to demo-spike-forms-row-layout March 3, 2023 12:55 Destroyed
@pinussilvestrus pinussilvestrus force-pushed the spike-forms-row-layout branch from 852f582 to 68436d5 Compare March 3, 2023 15:03
@github-actions github-actions bot temporarily deployed to demo-spike-forms-row-layout March 3, 2023 15:04 Destroyed
@github-actions github-actions bot temporarily deployed to demo-spike-forms-row-layout March 9, 2023 09:41 Destroyed
@pinussilvestrus pinussilvestrus force-pushed the spike-forms-row-layout branch from 1ebbe93 to 85c68a0 Compare March 9, 2023 09:41
@github-actions github-actions bot temporarily deployed to demo-spike-forms-row-layout March 9, 2023 09:41 Destroyed
@github-actions github-actions bot temporarily deployed to demo-spike-forms-row-layout March 9, 2023 14:29 Destroyed
@pinussilvestrus pinussilvestrus force-pushed the spike-forms-row-layout branch from 80431e3 to b4a0216 Compare March 10, 2023 08:20
@github-actions github-actions bot temporarily deployed to demo-spike-forms-row-layout March 10, 2023 08:20 Destroyed
@pinussilvestrus pinussilvestrus force-pushed the spike-forms-row-layout branch from b4a0216 to 45325a3 Compare March 10, 2023 08:42
@github-actions github-actions bot temporarily deployed to demo-spike-forms-row-layout March 10, 2023 08:42 Destroyed
@pinussilvestrus pinussilvestrus force-pushed the spike-forms-row-layout branch from 45325a3 to 4709c3d Compare March 10, 2023 09:00
@github-actions github-actions bot temporarily deployed to demo-spike-forms-row-layout March 10, 2023 09:00 Destroyed
@github-actions github-actions bot temporarily deployed to demo-spike-forms-row-layout March 10, 2023 13:46 Destroyed
@pinussilvestrus pinussilvestrus force-pushed the spike-forms-row-layout branch from 2e8b870 to 6e75c59 Compare March 10, 2023 14:40
@github-actions github-actions bot temporarily deployed to demo-spike-forms-row-layout March 10, 2023 14:40 Destroyed
@pinussilvestrus pinussilvestrus force-pushed the spike-forms-row-layout branch from 6e75c59 to 86d5abb Compare March 10, 2023 15:00
@github-actions github-actions bot temporarily deployed to demo-spike-forms-row-layout March 10, 2023 15:00 Destroyed
@pinussilvestrus pinussilvestrus force-pushed the spike-forms-row-layout branch 2 times, most recently from a26712d to beaf29f Compare March 13, 2023 09:38
@pinussilvestrus
Copy link
Contributor Author

@RomanKostka I made some adjustments, let me know what you think.

the drag row icon could have a larger area.
it is really hard to hit it with the mouse at the moment. An area like this would be super nice

I fixed that by giving the area a bit more space (32x32px).

when changing the state of a component from idle to dragged it seems that the component + border is jumping a bit around (~1 or 2 px) - not sure why this happens but it would be so clean if that would not happen.

This is not super easy to fix as a) the drop preview is a fixed element with fixed styles, and b) the form fields are different in their styling. I would not go for implementing drop previews for each individual form element, but I adjusted the height and margins so that it shouldn't jump around now. Still, it could not be super perfect on some occasions.

i realised that the drop-preview of single components has a different height then the standard component. would be nice if we match the drop-preview to the standard height. currently it produces some weird look and feel

This is fixed with the preview adjustments mentioned above

when trying to place components into 'full' rows the 'not-able-to-drop' indicator is jumping around unexpectedly

dragula decides where to place the drop previews given on some coordinates calculation. I am not sure whether we can make this even more fine graines so I guess we have to deal with that.

Copy link
Contributor

@vsgoulart vsgoulart left a comment

Choose a reason for hiding this comment

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

A small problem I found:

The cursor seems to be a bit weird, it only shows the dragging cursor if you grab on the left icon for the entire row. I think it should be any time you drag any element

@pinussilvestrus pinussilvestrus force-pushed the spike-forms-row-layout branch from 9a0575b to 4cefaf9 Compare March 27, 2023 14:32
@github-actions github-actions bot temporarily deployed to demo-spike-forms-row-layout March 27, 2023 14:33 Destroyed
Copy link

@RomanKostka RomanKostka left a comment

Choose a reason for hiding this comment

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

awesome!

i found some small misalignments:

  1. the drop preview area is now off to the left side - could be because of the larger drag row indicator?
    image

  2. the drop preview border is overlapping with the select border - not important but a detail i saw
    image

  3. (future topic) we should have a look into the sizes of each component. It feels weird having elements with different sizes when they still need the same visual space in a row.
    image

@pinussilvestrus
Copy link
Contributor Author

The cursor seems to be a bit weird, it only shows the dragging cursor if you grab on the left icon for the entire row. I think it should be any time you drag any element

I think this makes sense, the only thing I could imagine is that it's quite weird that for most of the canvas, the dragging cursor will be active now. But semantic-wise, it makes much sense. /cc @RomanKostka

@pinussilvestrus pinussilvestrus added needs review Review pending and removed in progress Currently worked on labels Mar 28, 2023
@github-actions github-actions bot temporarily deployed to demo-spike-forms-row-layout March 28, 2023 07:28 Destroyed
@github-actions github-actions bot temporarily deployed to demo-spike-forms-row-layout March 28, 2023 09:01 Destroyed
@github-actions github-actions bot temporarily deployed to demo-spike-forms-row-layout March 28, 2023 09:24 Destroyed
@pinussilvestrus
Copy link
Contributor Author

@vsgoulart @RomanKostka I pushed some changes and adjusted most of the things you mentioned (otherwise, I left a comment). Let's hope we can merge it now 👍 (note that we will have chances for visual updates anyway with the follow-ups).

Copy link

@RomanKostka RomanKostka left a comment

Choose a reason for hiding this comment

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

looks good to me - lets iterate on the visuals after the release.

two things we should take care of in next iterations:
the 'not allowed' curser disappears for some components.
also the space between the rows seem a bit large and could be reduced.

Screen.Recording.2023-03-28.at.12.36.52.mov

@github-actions github-actions bot temporarily deployed to demo-spike-forms-row-layout March 28, 2023 11:07 Destroyed
@pinussilvestrus
Copy link
Contributor Author

also the space between the rows seem a bit large and could be reduced.

I reduced it a bit, still, we need some space to make it easy to drag between rows.

For the cursor problem I opened a follow up: #576

@pinussilvestrus pinussilvestrus merged commit 9092a5f into develop Mar 28, 2023
@pinussilvestrus pinussilvestrus deleted the spike-forms-row-layout branch March 28, 2023 13:24
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement implicit row-based layout solution
5 participants