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

Element ratio lock and padding lock not persistent #1024

Closed
swissspidy opened this issue Apr 3, 2020 · 7 comments · Fixed by #1075
Closed

Element ratio lock and padding lock not persistent #1024

swissspidy opened this issue Apr 3, 2020 · 7 comments · Fixed by #1075
Assignees
Labels
P0 Critical, drop everything Type: Bug Something isn't working UX Needed

Comments

@swissspidy
Copy link
Collaborator

Bug Description

When I unselect an element and select it again (or reload the editor), I notice that the lock is enabled again.

Expected Behaviour

When I disable padding lock, I expect this to be a persistent setting.
When I disable aspect ratio lock, I expect this to be a persistent setting.

Steps to Reproduce

  1. Create new text element
  2. Disable padding lock
  3. Add some horizontal padding, but no vertical one
  4. Element updates as expected
  5. (Continue editing other elements)
  6. Edit text element again
  7. Change horizontal padding
  8. Notice how vertical padding now suddenly changes again

Screenshots

lock-ratio-persist


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance Criteria

QA Instructions

@swissspidy swissspidy added the Type: Bug Something isn't working label Apr 3, 2020
@dvoytenko
Copy link
Contributor

/cc @miina

@miina
Copy link
Contributor

miina commented Apr 3, 2020

I wonder if this is something that should this be persistent between reloads as well e.g. as in #825? Or would this be per element persistent until the editor is reloaded?

@dvoytenko
Copy link
Contributor

I think the padding specifically is more of a function of the current values. If horizontal === vertical, then the initial state of the lock should be on. Otherwise, it should be off.

@jauyong jauyong added the P0 Critical, drop everything label Apr 5, 2020
@jauyong jauyong removed their assignment Apr 5, 2020
@ndev1991 ndev1991 self-assigned this Apr 6, 2020
@dvoytenko
Copy link
Contributor

/cc @pbakaus it looks like my suggestion was a bad one. What do we want then here? Do we want lock state to be persisted in the data model? Or just always start on/off?

@pbakaus
Copy link
Contributor

pbakaus commented Apr 11, 2020

@dvoytenko just for posterity, the answer is we persist.

@pbakaus pbakaus added this to the 1.0 Alpha milestone Apr 11, 2020
@ndev1991
Copy link
Contributor

@pbakaus @dvoytenko We need to confirm the behavior when multiple elements selected.
When we have multiple element selected with different lock ratio value, we assume it as true and based on that calculate the size and padding.

@dvoytenko suggested that we need to have separate calculation on each element lock ratio value.
My question is if we need to separate the each calculation based on each element lock ratio for size and padding. And in that case what would be the status of lock ratio toggle?
Should it be disabled status?
/cc @samitron7

@dvoytenko
Copy link
Contributor

@dvoytenko suggested that we need to have separate calculation on each element lock ratio value.

I also chimed in on the other thread and for posterity: I think both options are very possible. But I actually leaning toward your initial intuition now, and use the common lock ratio. What's most important though: whichever way we go, we should stay consistent. So, assuming a multiple selection with different lock rations and changing the width, I see two options:

  1. Use per-element ratio and update height in one element, but not in the other. That means the width will show the common value (e.g. "100") and the height will be "multiple values".
  2. Use the common ratio. E.g. if the common ratio is "locked", we update height in both elements. But then we should also set ratio to "locked" for both elements in the data model.

This way we preserve the consistency. The width, height, and lock are all as they were last updated.

And again, I'm leaning to the second option at this time.

@swissspidy swissspidy modified the milestones: 1.0 Alpha, 1.0 Beta Apr 14, 2020
@kmyram kmyram removed this from the 1.0 Beta milestone Apr 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 Critical, drop everything Type: Bug Something isn't working UX Needed
Projects
None yet
7 participants