-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Components: Add Card Component #17963
Changes from 31 commits
ce0a22a
643d02f
c867811
93be05a
90a1bcf
66a869a
20fcdfd
3963db1
329218b
45a1f31
504169f
6324887
084a1e2
e93de65
05f27c1
7441e2e
0ec36f1
96ff8bd
4534ed4
efe6ea8
6ebe575
bfde122
1d24ec9
a8bd44f
9876eba
468097e
7df0507
3378c86
599324e
287b48e
1fe7528
af39009
23fbe54
32ae38a
3e596fd
9d26f28
0cca7bc
b9f852d
4ae4dcb
344e8b0
bb54a62
f40a890
ca8aad4
8559de5
ca80cd2
66b4421
dbe16a2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,7 @@ | |
"classnames": "^2.2.5", | ||
"clipboard": "^2.0.1", | ||
"dom-scroll-into-view": "^1.2.1", | ||
"hex-rgb": "^4.1.0", | ||
"lodash": "^4.17.15", | ||
"memize": "^1.0.5", | ||
"moment": "^2.22.1", | ||
|
@@ -43,6 +44,7 @@ | |
"react-dates": "^17.1.1", | ||
"react-spring": "^8.0.20", | ||
"rememo": "^3.0.0", | ||
"styled-components": "^4.4.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you know if this has any noticeable impact on bundle size? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not tiny, but it's not terribly large either (e.g. moment.js) It's currently at: https://bundlephobia.com/[email protected] The team is reducing the library size by a lot (and making it faster) in v5: One of the requirements is that you must be using React 16.8, for hook support. Thankfully, it looks like Gutenberg + |
||
"tinycolor2": "^1.4.1", | ||
"uuid": "^3.3.2" | ||
}, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hex-rgb
uses modern JavaScript (const
,let
, etc.) and is not transpiled to be able to run in older browsers.tl;dr: Including this library has broken the editor in Internet Explorer.
Apparently this is an intentional choice on their part: sindresorhus/hex-rgb#10 (comment)
If it's something we plan to use, it would need to be transpiled as well. I don't recall having had to deal with this for any other dependencies, and it would probably require some work to adapt into our build tooling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! Long term I think its probably good idea to support this in our tooling just as CRA, Parcel and other tools in the JavaScript ecosystem have done. Switching to
polished
'sparseToRgb
or similar fn might be a quick fix.Edit: I'm working on the quick fix now
Edit: The quick fix is here #18681
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, thinking about the longer-term fix, I saw this on Twitter the other day and it seems that not compiling (or only compiling so-far) is closer to becoming a defacto standard (yay for smaller bundle sizes for apps only supporting modern browsers!).
https://twitter.com/_developit/status/1196393947989512193
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR for updating the build tooling #18798