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

Bypass css text #2811

Merged
merged 2 commits into from
Jan 17, 2023
Merged

Bypass css text #2811

merged 2 commits into from
Jan 17, 2023

Conversation

pygy
Copy link
Member

@pygy pygy commented Jan 16, 2023

Shave a few bytes by setting element.style directly, rather than element.style.cssText, since the former is now supported everywhere relevant.

Description

Motivation and Context

How Has This Been Tested?

Types of changes

  • Internal tweak (non-breaking change which saves a few bytes of payload).
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation change

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@pygy pygy marked this pull request as ready for review January 16, 2023 10:46
@pygy
Copy link
Member Author

pygy commented Jan 16, 2023

This is a small cleanup before the #2809 fix lands (as a separate PR because auto changelog). The #2809 fix is in the pipeline once this is merged.

I'm about to do some cleanup such that the docs linter stops complaining, then I'll start doing substantial work.

Copy link
Member

@dead-claudia dead-claudia left a comment

Choose a reason for hiding this comment

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

Probably an old IE-ism or Safari-ism I've long forgotten about. Also, let's be honest: most of us just do it this way anyways nowadays.

@pygy
Copy link
Member Author

pygy commented Jan 17, 2023

It was for legacy support, MDN used to recommend not setting style because of it, but that advice has long been deprecated.

@pygy pygy merged commit c1fc1de into MithrilJS:next Jan 17, 2023
@JAForbes JAForbes mentioned this pull request Jan 17, 2023
kfule added a commit to kfule/mithril.js that referenced this pull request Feb 4, 2025
This is a leftover from MithrilJS#2811.
dead-claudia added a commit that referenced this pull request Feb 8, 2025
…Style() (#2988)

* setAttr/removeAttr: remove `key === "is"`, fixes #2799

This allows the "is" attribute to be set or removed like any other attribute.

* swap set and removal order of attributes/style properties

* bypass css text

This is a leftover from #2811.

* Vnode: add vnode.is to handle is-element

This seems to give better performance than simply adding a `(old.attrs && old.attrs.is) == (vnode.attrs && vnode.attrs.is)` evaluation to updateNode().

* refactor isEmpty()

* move `old.is === vnode.is` evaluation to the top of updateNode()

It seems better to evaluate `old.is === vnode.is` before process that assumes node update.

* revert isEmpty() back

* hasPropertyKey: evaluate `vnode.is` instead of `vnode.attrs.is`

* execSelector: use `attrs.is != null` instead of `hasOwn.call(attrs, "is")`

* Vnode: initialize `is` with undefined

* add manual test

* Update hyperscript.js

Signed-off-by: Claudia Meadows <[email protected]>

---------

Signed-off-by: Claudia Meadows <[email protected]>
Co-authored-by: Claudia Meadows <[email protected]>
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.

2 participants