accordproject / web-components

React Components for Accord Project
Apache License 2.0
117 stars 94 forks source link

style(ui-contract-editor): Drag-Drop Handle Resize - I137 #277

Closed K-Kumar-01 closed 3 years ago

K-Kumar-01 commented 3 years ago

Signed-off-by: k-kumar-01 kushalkumargupta4@gmail.com

Closes #137

Styling changes applied to the drag & drop handle so that short clauses don't have large space.

Changes

Screenshots or Video

Screenshot from 2021-03-03 00-37-03

Author Checklist

K-Kumar-01 commented 3 years ago

@Michael-Grover

It looks like there are 8 circles in the code, but only 6 are visible. Can we delete the two extra ones in the code?

I tried deleting them but it caused some styling issues initially. Also since they were not hampering with styles when left untouched I let them be there

it looks like the distance between the dots and the bottom arrow is greater than the distance between the dots and the top arrow. I think we should make these distances the same.

This might cause some increase in the space

It looks like the bottom arrow is now colliding with the clause template's outline at the bottom. I think we should move the buttons up to be vertically centered inside the space

I will try to see if I can find a solution to this

K-Kumar-01 commented 3 years ago

@Michael-Grover I changed some more styles. Screenshot from 2021-03-04 01-04-50

Zoomed. Screenshot from 2021-03-04 01-07-04

Are position and arrow correct now? (Yellow background only for testing purpose)

Also, the dots are causing an issue when I remove the code. So can it be kept as it is?

Michael-Grover commented 3 years ago

@K-Kumar-01 this looks good to me, thanks. This change isn't in Netlify yet, right? I looked at Netlify and the arrow is still intersecting with the clause template border

image

K-Kumar-01 commented 3 years ago

@K-Kumar-01 this looks good to me, thanks. This change isn't in Netlify yet, right? I looked at Netlify and the arrow is still intersecting with the clause template border

image

No I have not yet pushed the code. I wanted a review so I just shared images. I will make a pr ASAP.

K-Kumar-01 commented 3 years ago

@Michael-Grover I have made the changes in the PR. Let me know if there are any changes required.

Michael-Grover commented 3 years ago

@DianaLease

Looks good! Added one minor comment.

I can't find the comment you added. Is there anything you think should be changed?

@K-Kumar-01 it looks great to me, thanks! I think we should wait for @DianaLease or another engineer's review to merge.

Michael-Grover commented 3 years ago

@DianaLease never mind, I see your comment was resolved. It looks like this PR still requires an engineer approval before it can be merged though.