Element lock ratio for size and padding should be stored to data model

[ndev1991]

Fixes #1024

Add aspectLockRation to element as true, and padding locked to text element as true. If multiple element selected with different ratio it will be as true.

[github-actions]

Size Change: +133 B (0%)

Total Size: 869 kB

Filename	Size	Change
assets/js/edit-story.js	444 kB	+103 B (0%)
assets/js/stories-dashboard.js	419 kB	+30 B (0%)

ℹ️ View Unchanged

Filename	Size	Change
assets/css/edit-story.css	3.04 kB	0 B
assets/css/stories-dashboard.css	3.01 kB	0 B

_{compressed-size-action}

[ndev1991]

@pbakaus @dvoytenko @miina @barklund When we select the element with same horizontal and vertical padding the initial padding lock is enabled. If we have different horizontal and vertical padding initially then the padding lock will be opened. One thing here is when the padding lock opened and we input the same value on horizontal and vertical then the padding lock enabled.

Would you confirm if we need to keep padding lock open in this case?

[miina]

@ndev1991 I'd expect that the padding lock based on the padding value should be the default (e.g. on refresh), however, the user should still be able to unlock/lock the padding as they'd like, without it automatically changing the value.

Meaning that the initial padding lock state value could be set by padding.h === padding.v, however, if the user changes it manually, it shouldn't automatically change back.

Otherwise, the following could happen for example: if the user unlocks the padding, writes 1 to one field and starts typing 10 to the second field, then after typing the initial 1 to the second field, it already gets locked again.

[ndev1991]

@ndev1991 I'd expect that the padding lock based on the padding value should be the default (e.g. on refresh), however, the user should still be able to unlock/lock the padding as they'd like, without it automatically changing the value.

Meaning that the initial padding lock state value could be set by padding.h === padding.v, however, if the user changes it manually, it shouldn't automatically change back.

Otherwise, the following could happen for example: if the user unlocks the padding, writes 1 to one field and starts typing 10 to the second field, then after typing the initial 1 to the second field, it already gets locked again.

@miina Actually it doesn't happen in that way. If the user unlocks the padding and the h is 10 and the input v 100 is fine. but if the user input the h as 10 and input 10 for v and click h again, then the padding locks

[miina]

@ndev1991 Just tested it out, it does happen this way currently:

When there is 1 in one field and user starts typing 10 in the other field, then due to the first character of 10 being 1, a match is found and the lock put on again.

In any case -- the user's choice should remain and not change back unexpectedly, the paddings being equal is relevant for the default only.

[pbakaus]

IMHO the padding lock should never lock itself on user input of values. Just because they temporarily appear to be the same (10, 10) does not mean the user indicated that she wants them locked.

[dvoytenko]

Hmm. Sounds like this was a horrible suggestion by me. But what should be the behavior? Do we want to make the lock persistent in the data model then?

[swissspidy]

That‘d be a possibility, yes.

[ndev1991]

@pbakaus @swissspidy @dvoytenko We need to add padding lock persistent in the data model. Would you confirm?

[ndev1991]

@miina @dvoytenko Added paddingLockOpen to element property so we can solve above problems.

[pbakaus]

yes, padding lock should be persistent in the data model. Thanks!

[barklund]

Would prefer a rename, otherwise looks good.

[miina]

@ndev1991 The same should also go for width/height ratio lock then, I assume.

cc @pbakaus

[dvoytenko]

@miina @barklund And aspect ratio added by using the same format and nits!

Did we confirm we need lock for aspect? Last time I asked @samitron7 she didn't want one. /cc @pbakaus as well.

[ndev1991]

@dvoytenko @pbakaus @swissspidy Here is the current workflow of padding lock and aspect ratio.
Default element has lock aspect ratio status. Default text has lock padding status.
When there are multiple elements selected with different aspect ratio and padding ratio, assume it as true status and make the same changes for each size and padding calculation.

If we want to go for each element, lock toggle should be disabled status?
And also for sizePosition calculation, orgRatio is based on common values. Do we need to have separate calculation for each elements? /cc @samitron7

[dvoytenko]

@pbakaus @dvoytenko @miina @barklund When we select the element with same horizontal and vertical padding the initial padding lock is enabled. If we have different horizontal and vertical padding initially then the padding lock will be opened. One thing here is when the padding lock opened and we input the same value on horizontal and vertical then the padding lock enabled.

This is a good point. No matter how we approach this, from the data model point of view: it makes no sense to allow "padding lock" if H and V values are different - otherwise we won't be able to change them. And probably we just need to ensure there're no inconsistencies and it would be enough.

[ndev1991]

@dvoytenko @barklund Following the second option, it uses the common lock ratio for padding and size. When there are multiple values, assume it as locked and update the lock ratio for element also.

[barklund]

@dvoytenko should this have a migration? If width/height are still proportional to originalWidth/originalHeight, we migrate to locked? And similarly, if paddings are still equal, we migrate to locked? Or should we just assume all old stories now to be unlocked, and it's not really a big issue?

[ndev1991]

@dvoytenko should this have a migration? If width/height are still proportional to originalWidth/originalHeight, we migrate to locked? And similarly, if paddings are still equal, we migrate to locked? Or should we just assume all old stories now to be unlocked, and it's not really a big issue?

We can have basic thing on it. Like rawLockRatio === undefined, we can assume to be true?

[dvoytenko]

Very close. But some questions. I'll also loop in Paul on this.

[dvoytenko]

Just one update requested.

[barklund]

I just have a few comments about tests, otherwise things look really good. Please think about good simple test descriptions, that don't go into unnecessary technical details and try to use labels or visual texts rather than test ids whenever possible.