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

Added missing attribute transform-origin #26115

Closed
wants to merge 4 commits into from
Closed

Conversation

arav-ind
Copy link
Contributor

@arav-ind arav-ind commented Feb 6, 2023

Summary

This PR resolves #26112

While trying to use the attribute transformOrigin, we get the following warning.
'React does not recognize the transformOrigin prop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercase transformorigin instead. If you accidentally passed it from a parent component, remove it from the DOM element.'

This issue was already raised by #11265, at that point 'transform-origin' was not a valid SVG attribute.

As mentioned in #26112, transform-origin is now a proper SVG attribute (seen here).
So added the same to possibleStandardNames.

How did you test this change?

  1. The issue can be seen here

  2. I made the changes and run yarn build react/index,react-dom/index --type=UMD. Please find the bundle screenshot.
    image

  3. Added the following changes in fixtures/packaging/babel-standalone/dev.html, to verify the issue.

<script type="text/babel">
      ReactDOM.render(
        <div>
        <svg height="512" width="512" >
          <g>
            <path
                d="M254,65.51c0.666,-0.007 1.333,-0.01 2,-0.01c105.14,0 190.5,85.36 190.5,190.5c0,105.14 -85.36,190.5 -190.5,190.5c-105.14,0 -190.5,-85.36 -190.5,-190.5c0,-104.138 84.741,-188.884 188.5,-190.49l0,25.002c-89.96,1.602 -163.5,75.148 -163.5,165.488c0,91.342 74.158,165.5 165.5,165.5c91.342,0 165.5,-74.158 165.5,-165.5c0,-91.342 -74.158,-165.5 -165.5,-165.5c-0.668,0 -1.334,0.004 -2,0.012l0,-25.002Z"
                fill="#d3d3d3"
              />
             <g transformOrigin="256 256" transform="rotate(30)" >
              <path
                  d="M256,65.5c105.14,0 190.5,85.36 190.5,190.5l0,0c0,6.901 -5.603,12.504 -12.504,12.504c-6.901,0 -12.504,-5.603 -12.504,-12.504l0,0c0,-0.141 0.003,-0.282 0.007,-0.422c-0.227,-91.003 -74.062,-164.843 -165.064,-165.077c-0.144,0.005 -0.289,0.007 -0.435,0.007c-6.901,0 -12.504,-5.603 -12.504,-12.504l0,0c0,-6.901 5.603,-12.504 12.504,-12.504l0,0Z"
                  fill="#000"
                />
            </g>
          </g>
        </svg>
    	</div>,
      document.getElementById('container')
      );
    </script>
  1. Now in the console, we don't get to see the error when we are using the attribute transformOrigin.
  2. Screenshot of the console attached below.
    image

@react-sizebot
Copy link

react-sizebot commented Feb 7, 2023

Comparing: 78d2e9e...cc2c487

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 154.82 kB 154.82 kB = 49.11 kB 49.11 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 156.82 kB 156.82 kB = 49.72 kB 49.72 kB
facebook-www/ReactDOM-prod.classic.js = 533.52 kB 533.52 kB = 94.98 kB 94.98 kB
facebook-www/ReactDOM-prod.modern.js = 518.79 kB 518.79 kB = 92.83 kB 92.83 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against cc2c487

Copy link
Collaborator

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

This is missing an update to DOMProperty to ensure transformOrigin is written as transform-origin to the DOM.

Made the necessary local changes but since you made the PR from your default branch instead of a new one, I can't push them. In the future, please create a new branch, make the PR from that branch and allow maintainers to edit.

patches
commit b0af6973c6fb899b989b5421c890dbc56ee784d0
Author: eps1lon <silbermann.sebastian@gmail.com>
Date:   Wed Feb 8 16:36:19 2023 +0100

    Ensure correct attribute is written

diff --git a/fixtures/attribute-behavior/AttributeTableSnapshot.md b/fixtures/attribute-behavior/AttributeTableSnapshot.md
index 55928a644..559a82fa4 100644
--- a/fixtures/attribute-behavior/AttributeTableSnapshot.md
+++ b/fixtures/attribute-behavior/AttributeTableSnapshot.md
@@ -1,4 +1,4 @@
-## `about` (on `<div>` inside `<div>`)
+## `about` (on `<div>` inside `<div>`)
 | Test Case | Flags | Result |
 | --- | --- | --- |
 | `about=(string)`| (changed)| `"a string"` |
@@ -11401,23 +11401,23 @@
 ## `transformOrigin` (on `<svg>` inside `<div>`)
 | Test Case | Flags | Result |
 | --- | --- | --- |
-| `transformOrigin=(string)`| (initial)| `<null>` |
-| `transformOrigin=(empty string)`| (initial)| `<null>` |
-| `transformOrigin=(array with string)`| (initial)| `<null>` |
-| `transformOrigin=(empty array)`| (initial)| `<null>` |
-| `transformOrigin=(object)`| (initial)| `<null>` |
-| `transformOrigin=(numeric string)`| (initial)| `<null>` |
-| `transformOrigin=(-1)`| (initial)| `<null>` |
-| `transformOrigin=(0)`| (initial)| `<null>` |
-| `transformOrigin=(integer)`| (initial)| `<null>` |
-| `transformOrigin=(NaN)`| (initial, warning)| `<null>` |
-| `transformOrigin=(float)`| (initial)| `<null>` |
+| `transformOrigin=(string)`| (changed)| `"a string"` |
+| `transformOrigin=(empty string)`| (changed)| `<empty string>` |
+| `transformOrigin=(array with string)`| (changed)| `"string"` |
+| `transformOrigin=(empty array)`| (changed)| `<empty string>` |
+| `transformOrigin=(object)`| (changed)| `"result of toString()"` |
+| `transformOrigin=(numeric string)`| (changed)| `"42"` |
+| `transformOrigin=(-1)`| (changed)| `"-1"` |
+| `transformOrigin=(0)`| (changed)| `"0"` |
+| `transformOrigin=(integer)`| (changed)| `"1"` |
+| `transformOrigin=(NaN)`| (changed, warning)| `"NaN"` |
+| `transformOrigin=(float)`| (changed)| `"99.99"` |
 | `transformOrigin=(true)`| (initial, warning)| `<null>` |
 | `transformOrigin=(false)`| (initial, warning)| `<null>` |
-| `transformOrigin=(string 'true')`| (initial)| `<null>` |
-| `transformOrigin=(string 'false')`| (initial)| `<null>` |
-| `transformOrigin=(string 'on')`| (initial)| `<null>` |
-| `transformOrigin=(string 'off')`| (initial)| `<null>` |
+| `transformOrigin=(string 'true')`| (changed)| `"true"` |
+| `transformOrigin=(string 'false')`| (changed)| `"false"` |
+| `transformOrigin=(string 'on')`| (changed)| `"on"` |
+| `transformOrigin=(string 'off')`| (changed)| `"off"` |
 | `transformOrigin=(symbol)`| (initial, warning)| `<null>` |
 | `transformOrigin=(function)`| (initial, warning)| `<null>` |
 | `transformOrigin=(null)`| (initial)| `<null>` |
diff --git a/packages/react-dom-bindings/src/shared/DOMProperty.js b/packages/react-dom-bindings/src/shared/DOMProperty.js
index 1c73df1a6..9e7b7bf67 100644
--- a/packages/react-dom-bindings/src/shared/DOMProperty.js
+++ b/packages/react-dom-bindings/src/shared/DOMProperty.js
@@ -521,6 +521,7 @@ const capitalize = (token: string) => token[1].toUpperCase();
   'text-anchor',
   'text-decoration',
   'text-rendering',
+  'transform-origin',
   'underline-position',
   'underline-thickness',
   'unicode-bidi',


commit 270a6ccd52a73d6b3aa3de95c62dbf9d55e50c23
Author: eps1lon <silbermann.sebastian@gmail.com>
Date:   Wed Feb 8 16:13:52 2023 +0100

    Add to attribute-behavior fixture

diff --git a/fixtures/attribute-behavior/AttributeTableSnapshot.md b/fixtures/attribute-behavior/AttributeTableSnapshot.md
index 93ae5b010..55928a644 100644
--- a/fixtures/attribute-behavior/AttributeTableSnapshot.md
+++ b/fixtures/attribute-behavior/AttributeTableSnapshot.md
@@ -11373,6 +11373,56 @@
 | `transform=(null)`| (initial)| `[]` |
 | `transform=(undefined)`| (initial)| `[]` |
 
+## `transform-origin` (on `<svg>` inside `<div>`)
+| Test Case | Flags | Result |
+| --- | --- | --- |
+| `transform-origin=(string)`| (changed, warning)| `"a string"` |
+| `transform-origin=(empty string)`| (changed, warning)| `<empty string>` |
+| `transform-origin=(array with string)`| (changed, warning)| `"string"` |
+| `transform-origin=(empty array)`| (changed, warning)| `<empty string>` |
+| `transform-origin=(object)`| (changed, warning)| `"result of toString()"` |
+| `transform-origin=(numeric string)`| (changed, warning)| `"42"` |
+| `transform-origin=(-1)`| (changed, warning)| `"-1"` |
+| `transform-origin=(0)`| (changed, warning)| `"0"` |
+| `transform-origin=(integer)`| (changed, warning)| `"1"` |
+| `transform-origin=(NaN)`| (changed, warning)| `"NaN"` |
+| `transform-origin=(float)`| (changed, warning)| `"99.99"` |
+| `transform-origin=(true)`| (initial, warning)| `<null>` |
+| `transform-origin=(false)`| (initial, warning)| `<null>` |
+| `transform-origin=(string 'true')`| (changed, warning)| `"true"` |
+| `transform-origin=(string 'false')`| (changed, warning)| `"false"` |
+| `transform-origin=(string 'on')`| (changed, warning)| `"on"` |
+| `transform-origin=(string 'off')`| (changed, warning)| `"off"` |
+| `transform-origin=(symbol)`| (initial, warning)| `<null>` |
+| `transform-origin=(function)`| (initial, warning)| `<null>` |
+| `transform-origin=(null)`| (initial, warning)| `<null>` |
+| `transform-origin=(undefined)`| (initial, warning)| `<null>` |
+
+## `transformOrigin` (on `<svg>` inside `<div>`)
+| Test Case | Flags | Result |
+| --- | --- | --- |
+| `transformOrigin=(string)`| (initial)| `<null>` |
+| `transformOrigin=(empty string)`| (initial)| `<null>` |
+| `transformOrigin=(array with string)`| (initial)| `<null>` |
+| `transformOrigin=(empty array)`| (initial)| `<null>` |
+| `transformOrigin=(object)`| (initial)| `<null>` |
+| `transformOrigin=(numeric string)`| (initial)| `<null>` |
+| `transformOrigin=(-1)`| (initial)| `<null>` |
+| `transformOrigin=(0)`| (initial)| `<null>` |
+| `transformOrigin=(integer)`| (initial)| `<null>` |
+| `transformOrigin=(NaN)`| (initial, warning)| `<null>` |
+| `transformOrigin=(float)`| (initial)| `<null>` |
+| `transformOrigin=(true)`| (initial, warning)| `<null>` |
+| `transformOrigin=(false)`| (initial, warning)| `<null>` |
+| `transformOrigin=(string 'true')`| (initial)| `<null>` |
+| `transformOrigin=(string 'false')`| (initial)| `<null>` |
+| `transformOrigin=(string 'on')`| (initial)| `<null>` |
+| `transformOrigin=(string 'off')`| (initial)| `<null>` |
+| `transformOrigin=(symbol)`| (initial, warning)| `<null>` |
+| `transformOrigin=(function)`| (initial, warning)| `<null>` |
+| `transformOrigin=(null)`| (initial)| `<null>` |
+| `transformOrigin=(undefined)`| (initial)| `<null>` |
+
 ## `type` (on `<button>` inside `<div>`)
 | Test Case | Flags | Result |
 | --- | --- | --- |
diff --git a/fixtures/attribute-behavior/src/attributes.js b/fixtures/attribute-behavior/src/attributes.js
index b84a0d11d..c0e5a133f 100644
--- a/fixtures/attribute-behavior/src/attributes.js
+++ b/fixtures/attribute-behavior/src/attributes.js
@@ -1981,6 +1981,16 @@ const attributes = [
     overrideStringValue:
       'translate(-10,-20) scale(2) rotate(45) translate(5,10)',
   },
+  {
+    name: 'transform-origin',
+    tagName: 'svg',
+    read: getSVGAttribute('transform-origin'),
+  },
+  {
+    name: 'transformOrigin',
+    tagName: 'svg',
+    read: getSVGAttribute('transform-origin'),
+  },
   {name: 'type', tagName: 'button', overrideStringValue: 'reset'},
   {
     name: 'type',

@arav-ind
Copy link
Contributor Author

arav-ind commented Feb 8, 2023

This is missing an update to DOMProperty to ensure transformOrigin is written as transform-origin to the DOM.

Made the necessary local changes but since you made the PR from your default branch instead of a new one, I can't push them. In the future, please create a new branch, make the PR from that branch and allow maintainers to edit.

patches

Hi.. Sorry for this. This is my first PR. I will make sure this does not happen in future. So, now I just need to integrate the patches to my main and update the PR. Is that correct?

@eps1lon
Copy link
Collaborator

eps1lon commented Feb 8, 2023

So, now I just need to integrate the patches to my main and update the PR. Is that correct?

Yep! Though I'd recommend, redoing it with the GitHub flow explained below for practice. Then I can push my patches directly.

Hi.. Sorry for this. This is my first PR.

No worries. Happy to assist. Let me know if https://docs.github.com/en/get-started/quickstart/github-flow#prerequisites helps or if there's anything unclear.

@arav-ind
Copy link
Contributor Author

arav-ind commented Feb 8, 2023

Yep! Though I'd recommend, redoing it with the GitHub flow explained below for practice. Then I can push my patches directly.

I have created a new PR as recommended. Please let me know if anything incorrect.
Thanks!

@eps1lon
Copy link
Collaborator

eps1lon commented Feb 9, 2023

Clsoing in favor of #26130

@eps1lon eps1lon closed this Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: transform-origin is not recognized as a valid attribute despite being in SVG specs
4 participants