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

Add data-lift-fixedeventattribute-<event-name> for each replaced event-attribute #1701

Merged
merged 4 commits into from
Jul 6, 2015

Conversation

andreak
Copy link
Member

@andreak andreak commented Apr 25, 2015

Add data-lift-fixedeventattribute- for each replaced event-attribute, with settings in LiftRules: LiftRules.includeFixedEventAttributesAsDataAttributes_? (default false)

…t-attribute, with settings in LiftRules: LiftRules.includeFixedEventAttributesAsDataAttributes_? (default false)
@Shadowfiend
Copy link
Member

Two broad thoughts on the feature:

  • Switch instead to data-lift-removed="<attribute name> <attribute name> <attribute name>".
  • Switch instead to adding a class lift-removed-<attribute> lift-removed-<attribute>.

Thought on both fronts is that having one attribute to represent these would be nice. Using a class is nice because there's such strong and optimized support for selection on classes in browsers.
Thoughts?

@fmpwizard fmpwizard added this to the 3.0-M6 milestone Apr 27, 2015
@andreak
Copy link
Member Author

andreak commented Jun 22, 2015

I somewhat think that "removed" is a little bit too general in this case so I propose using
data-lift-removedattributes="onclick onmouseover"

Ok?

@andreak
Copy link
Member Author

andreak commented Jun 22, 2015

A colleague of mine pointed out that the name should be data-lift-removed-attributesso that it becomes el.dataset.liftRemovedAttributes in javascript

* }
* </pre>
*/
@volatile var includeFixedEventAttributesAsDataAttributes_? = false
Copy link
Member

Choose a reason for hiding this comment

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

Two questions:

  • Should this be includeRemovedEventAttributesAsDataAttributes_??
  • Let's use Scaladoc markup rather than HTML (triple-backticks for multiline code blocks, single backticks for inline code blocks, empty lines for line breaks, etc).

Copy link
Member Author

Choose a reason for hiding this comment

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

  1. Yes, Fixed in last commit.
  2. I'm not able to make this change in a way that IntelliJ will recognize the markup and show the same as the HTML-version. Can you make the changes you suggest?

…dded configurable attribute-name for "data-lift-removed-attributes", set in LiftRules.removedEventAttributesAttributeName
@andreak
Copy link
Member Author

andreak commented Jun 24, 2015

I added a configurable setting, LiftRules.removedEventAttributesAttributeName = "data-lift-removed-attributes", so that one can change that if wanted.

@Shadowfiend
Copy link
Member

I'm going to merge the two LiftRules into attributeForRemovedEventAttributes and make that an Option, if that's cool by you—a None will simply not emit the removed attributes into the markup. Sound good?

We now have one LiftRules.attributeForRemovedEventAttributes, which can be None
to indicate no attribute should be emitted. The default is currently None.
@andreak
Copy link
Member Author

andreak commented Jul 3, 2015

That's of course much better, thanks!

@Shadowfiend
Copy link
Member

Schweet, give that a shot and see what you think!

@andreak
Copy link
Member Author

andreak commented Jul 4, 2015

Just tested it and it works great!
Merge it in:-)

@Shadowfiend
Copy link
Member

👍 I'll leave it here for another day or two in case anyone else wants to make some remarks and then :shipit: .

@Shadowfiend
Copy link
Member

All righty let's do it!

Shadowfiend added a commit that referenced this pull request Jul 6, 2015
Add LiftRules.attributeForRemovedEventAttributes

This PR adds a LiftRule, LiftRules.attributeForRemovedEventAttributes.
When we remove a JavaScript event from an element for separate
application via the page's JS file, if this rule is set, the attribute it specified
is populated with the names of the attributes that were removed from the
element. If no attributes were removed, no attribute is added to the element.

The default for the LiftRule is to be None, meaning this information is never
added to the output.
@Shadowfiend Shadowfiend merged commit 6b1429e into master Jul 6, 2015
@Shadowfiend Shadowfiend deleted the ajk_datalift-fixedattrs branch July 6, 2015 23:19
@andreak
Copy link
Member Author

andreak commented Jul 7, 2015

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants