-
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: Update Flex #28609
Components: Update Flex #28609
Conversation
Size Change: +1.3 kB (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
fce18c8
to
2a5ad25
Compare
2a5ad25
to
dd9e4a5
Compare
* | ||
* @example | ||
* ```jsx | ||
* import { Flex, FlexItem, FlexBlock, Text, View } from `@wordpress/components` |
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.
Minor update change the import path to components/ui
.
It's a bummer that we have to manually duplicate this stuff in multiple places :(.
For now, I wonder if it may be easier remove the inline examples until we can figure out some tooling to help automate things
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.
Alternatively we could just remove the import statements from the inline examples. They're probably the least important part of the examples.
@saramarcondes So far so good!! I'm testing this locally in Gutenberg, forcing the new I'm spotting a minor implementation issue.. but I think it can be easily resolved. To be specific, it's related to the current That being said.. I expect this to resolve by refactoring the In the meantime, we can add a patch fix to smoothen out P.S. Moving forward, it is unlikely for us to use |
Update: Alternatively... we could change the For context... Current implementation:
G2 implementation:
Technically they achieve the same thing. But I think the current implementation has less room for side-effects Update: Looked into things in the G2 repo. There's pros/cons to both Flex inner child gap strategies 😂 😭 . I think the newer G2 implementation offers more room for adjustments. I went through a bunch of places where this is used, in particular all For example, an input
For a Not impossible, just a lot more work. With all this said... I think it's better to keep the G2 Flex gap strategy, and make retroactive adjustments to areas that may be affected. From what I've seen, it's only the |
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.
🚀 from me!! Thank you @saramarcondes
Description
Add G2 flex component
How has this been tested?
Storybook
Typecheck
Types of changes
New feature
Checklist: