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.

Sign up for GitHub

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

Jump to bottom

Live updates to Button do not target correct DOM node #50

Closed
talesg opened this issue Oct 10, 2016 · 2 comments
Closed

Live updates to Button do not target correct DOM node #50

talesg opened this issue Oct 10, 2016 · 2 comments
Labels
bug
Milestone
1.6.1

Comments

@talesg
Copy link

talesg commented Oct 10, 2016

If padding is defined and some value from the "General" tab is changed (such as label or link), the padding is lost (although the recorded setting still remains).

To replicate:

  1. Add button
  2. Change padding-left to 50px
  3. Apply
  4. Edit the button
  5. Change link or label
  6. Verify that the left padding was lost

Obs: using latest Tailor version in one test site only. Didn't tested in any other site.

Other 2 Things About Padding/Margin:

  1. I think it would be much better if the default setting for Margin and Padding was individual instead of "all". Because if padding-left is set to 50px and i edit the button to change only that setting, by default it will be applied to all (top, bottom and right also).

  2. Although is interesting to have the freedom to set the unit i desire, it would be nicer if the default were (px), so if i add only the number 50 it automatically assumes it is 50px and applies the setting. And if i specify % or other, it will apply that setting.

Since is my first time communicating, let me just say: this page builder is awesome. Is the one that currently has the most potential, in my opinion (specially aiming for a simplified client-oriented front-end editing experience). Great work Andrew, congratulations and thanks for your support (in general, as i've been following Tailor-related issues around).
@andrew-worsfold
Copy link
Contributor

Thanks for raising this, @talesg (and for your kind words!).

The issue is that the live updates performed by JavaScript do not target the inner button element. This causes the padding to appear like margin (and the discrepancy between the live updates and those from the server which arise from changing things like the label).

I have a fix for this available which will be released in the next version (1.6.1.).

Regarding your other points:

  1. An assumption I made early on is that most people who were wanting to add/remove margin or padding on an element would be doing so evenly across all sides. I think you may be right in that this is more of an advanced user feature and it's therefore likely they would be making refinements of the sort you mentioned. I am happy to consider changing this default but would like to try and get feedback from some other users first regarding their usage of this setting.

  2. That's a good idea. I will look to include this in an upcoming version of Tailor!

Cheers,
Andrew.

@andrew-worsfold andrew-worsfold added this to the 1.6.1 milestone Oct 10, 2016
@andrew-worsfold andrew-worsfold changed the title Element "Button" loses property Padding after being edited Live updates to Button do not target correct DOM node Oct 10, 2016
andrew-worsfold added a commit that referenced this issue Oct 12, 2016
@andrew-worsfold
Tailor 1.6.1.
29de375 
Improved - Reduced the amount of layout meta information saved to the post, the selector for element child containers can now be specified when registering the element.
Fixed - Live updates to Button do not target correct DOM node [GitHub #50](https://github.com/andrew-worsfold/tailor/issues/50), cannot add multiple custom class names to an element [GitHub PR #51](https://github.com/andrew-worsfold/tailor/pull/51), fields added to the widget form by third-party plugins are not handled correctly [GitHub PR #52](https://github.com/andrew-worsfold/tailor/pull/52).
ManUtopiK added a commit to ManUtopiK/tailor that referenced this issue Oct 13, 2016
@ManUtopiK
Merge branch 'master' of https://github.com/andrew-worsfold/tailor in…
b5f1ccf 
…to Fix-widgets-controlChange

* 'master' of https://github.com/andrew-worsfold/tailor:
  Tailor 1.6.2. Fixed - Copy history entry created when copying using the Copy button.
  Tailor 1.6.2. Added - "Copy" button to elements.
  Tailor 1.6.1. Improved - Reduced the amount of layout meta information saved to the post, the selector for element child containers can now be specified when registering the element. Fixed - Live updates to Button do not target correct DOM node [GitHub Enclavely#50](https://github.com/andrew-worsfold/tailor/issues/50), cannot add multiple custom class names to an element [GitHub PR Enclavely#51](https://github.com/andrew-worsfold/tailor/pull/51), fields added to the widget form by third-party plugins are not handled correctly [GitHub PR Enclavely#52](https://github.com/andrew-worsfold/tailor/pull/52).
  Prevent null matches. (Enclavely#52)
  Allow multiple class on attribute class name. Prevent multiple whitespace and whitespace and the end of string; (Enclavely#51)

# Conflicts:
#	assets/js/dist/sidebar.js
#	assets/js/src/sidebar/components/controls/widget-form.js
@andrew-worsfold
Copy link
Contributor

Closing this issue as the bug has been resolved, however will look to include those recommendations in a future version of Tailor!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
1.6.1
Development

No branches or pull requests
2 participants
@talesg @andrew-worsfold