feat: add text as new component #26090
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chrisdholt
requested review from
EisenbergEffect,
nicholasrice and
a team
as code owners
📊 Bundle size reportUnchanged fixtures
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 18ec7f8:
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: 7c94cbd46051ea57bba4e8885c86e89967bb412c (build) |
chrisdholt
force-pushed
the
users/chhol/add-text-as-new-component
branch
2 times, most recently
from
fbbd80c
to
4e9bb34
Compare
|
@Hotell would be great if you can take a look at the ESM, storybook and package.json updates just to understand my note on issues with deduplication as I had issues getting things building again when making the necessary changes. Also, the build is failing on API extractor and I believe it has to do with which tsconfig is being referenced (perhaps)? |
packages/web-components/package.json
Outdated
| @@ -18,6 +19,12 @@ | |||
| "main": "dist/esm/index.js", | |||
| "types": "dist/web-components.d.ts", | |||
| "unpkg": "dist/web-components.min.js", | |||
| "exports": { | |||
| "./text/define": { | |||
There was a problem hiding this comment.
What is the use case for using this import vs package import?
I guess I am supposed to import components/define for all used components to register the custom elements and package import anything else?
Perhaps worth adding a readme with a brief Usage section.
There was a problem hiding this comment.
This allows one-off imports of individual components as each will have a unique path in the package.
import "@fluentui/web-components/text";There was a problem hiding this comment.
Following up on this, the above example doesn't represent what was originally in the PR, so revisiting this...I think one reason that we went with ./text/define which would be used as below was to leave room for the opportunity for a text specific path to export more than just the defined element. I think that is probably too complex and everything is available via the root of the package, so I've updated this to be available just as "text" per above because it's more intuitive than a "define" path IMO. We can adjust if need be, but this makes more sense than what was originally there (shown below).
import "@fluentui/web-components/text/define";
|
tooling support #26298
|
| * HTML Element: \<fluent-text\> | ||
| */ | ||
| export const definition = Text.compose({ | ||
| name: `${FluentDesignSystem.prefix}-text`, |
There was a problem hiding this comment.
I do not understand why the component definition is coupled with FluentDesignSystem.prefix.
We pass registry to definition.define() to support custom/multiple registries. In that case, wouldn't it make sense to pass the prefix to define() as well?
There was a problem hiding this comment.
Good question - Let me follow up on this. If you can send a mail I'll take it offline. This was a pattern we established awhile ago and I want to make sure I'm not missing something w/r/t our discussion around "defaultRegistry" vs current implementation.
| * HTML Attribute: weight | ||
| */ | ||
| @attr | ||
| weight: TextWeight; |
There was a problem hiding this comment.
How do we want to handle and communicate default values?
Just jsdocs? Or assign the default value? And use fromView in that case to prevent DOM pollution?
There was a problem hiding this comment.
Agreed we'll handle via jsDocs and we'll set defaults via CSS so we don't require reflecting attributes creating an unnecessarily complex DOM just for defaults. Boolean attr's should default to false which may require deltas as seen in this PR with wrap => nowrap.
Regarding fromView, I think that prevents setting via IDL attribute (js properties) so we should likely avoid for this instance.
| overflow: visible; | ||
| text-overflow: clip; | ||
| } | ||
| :host([truncate]) ::slotted(*), |
There was a problem hiding this comment.
FUIR9 does not add nowrap styles when truncate is set. It is users responsibility to set wrap={false} together with truncate:
We should unify.
There was a problem hiding this comment.
Good to know! I think this is likely just a change I made not knowing that and wondering why it wasn't working :)
| overflow: hidden; | ||
| } | ||
| :host([truncate]) ::slotted(*) { | ||
| display: block; |
There was a problem hiding this comment.
FUIR9 does not set display: block for truncate.
Hotell
reviewed
Jan 11, 2023
There was a problem hiding this comment.
TS 4.7 and ESM support for wc 3 is ready ! 🫡
- PR: chore: setup typescript 4.7 for web-components package #26299
- Integration repo: [DO NOT MERGE]: setup typescript 4.7 for web-components package #26298 - Please test it out ( to me that storybook looks bit broken )
miroslavstastny
approved these changes
Jan 11, 2023
chrisdholt
force-pushed
the
users/chhol/add-text-as-new-component
branch
from
4e9bb34
to
7e6d618
Compare
…pdate stories to leverage object.keys of options
…e to object.values
chrisdholt
force-pushed
the
users/chhol/add-text-as-new-component
branch
from
457d3a6
to
7fc847c
Compare
chrisdholt
force-pushed
the
users/chhol/add-text-as-new-component
branch
from
2c65bd2
to
18ec7f8
Compare
JustSlone
approved these changes
Jan 12, 2023
Hotell
pushed a commit
to Hotell/fluentui
that referenced
this pull request
Feb 9, 2023
* add text as a new component * update main.js to .cjs format for esm due to require syntax * update style rules to align with FUI react v9, export text options, update stories to leverage object.keys of options * export text, update node resolution to 16 and fix object.keys for size to object.values * prettier style files, may need to revert if it does not play nice with selectors * update display for host to ensure we can layout the element properly margin, etc... * update latest FAST packages * leverage ValuesOf type helper
Hotell
pushed a commit
to Hotell/fluentui
that referenced
this pull request
Feb 9, 2023
* add text as a new component * update main.js to .cjs format for esm due to require syntax * update style rules to align with FUI react v9, export text options, update stories to leverage object.keys of options * export text, update node resolution to 16 and fix object.keys for size to object.values * prettier style files, may need to revert if it does not play nice with selectors * update display for host to ensure we can layout the element properly margin, etc... * update latest FAST packages * leverage ValuesOf type helper
Hotell
pushed a commit
to Hotell/fluentui
that referenced
this pull request
Feb 9, 2023
* add text as a new component * update main.js to .cjs format for esm due to require syntax * update style rules to align with FUI react v9, export text options, update stories to leverage object.keys of options * export text, update node resolution to 16 and fix object.keys for size to object.values * prettier style files, may need to revert if it does not play nice with selectors * update display for host to ensure we can layout the element properly margin, etc... * update latest FAST packages * leverage ValuesOf type helper
chrisdholt
added a commit
that referenced
this pull request
Apr 29, 2024
* add text as a new component * update main.js to .cjs format for esm due to require syntax * update style rules to align with FUI react v9, export text options, update stories to leverage object.keys of options * export text, update node resolution to 16 and fix object.keys for size to object.values * prettier style files, may need to revert if it does not play nice with selectors * update display for host to ensure we can layout the element properly margin, etc... * update latest FAST packages * leverage ValuesOf type helper
radium-v
pushed a commit
to radium-v/fluentui
that referenced
this pull request
Apr 29, 2024
* add text as a new component * update main.js to .cjs format for esm due to require syntax * update style rules to align with FUI react v9, export text options, update stories to leverage object.keys of options * export text, update node resolution to 16 and fix object.keys for size to object.values * prettier style files, may need to revert if it does not play nice with selectors * update display for host to ensure we can layout the element properly margin, etc... * update latest FAST packages * leverage ValuesOf type helper
radium-v
pushed a commit
to radium-v/fluentui
that referenced
this pull request
Apr 29, 2024
* add text as a new component * update main.js to .cjs format for esm due to require syntax * update style rules to align with FUI react v9, export text options, update stories to leverage object.keys of options * export text, update node resolution to 16 and fix object.keys for size to object.values * prettier style files, may need to revert if it does not play nice with selectors * update display for host to ensure we can layout the element properly margin, etc... * update latest FAST packages * leverage ValuesOf type helper
radium-v
pushed a commit
to radium-v/fluentui
that referenced
this pull request
Apr 30, 2024
* add text as a new component * update main.js to .cjs format for esm due to require syntax * update style rules to align with FUI react v9, export text options, update stories to leverage object.keys of options * export text, update node resolution to 16 and fix object.keys for size to object.values * prettier style files, may need to revert if it does not play nice with selectors * update display for host to ensure we can layout the element properly margin, etc... * update latest FAST packages * leverage ValuesOf type helper
radium-v
pushed a commit
that referenced
this pull request
Apr 30, 2024
* add text as a new component * update main.js to .cjs format for esm due to require syntax * update style rules to align with FUI react v9, export text options, update stories to leverage object.keys of options * export text, update node resolution to 16 and fix object.keys for size to object.values * prettier style files, may need to revert if it does not play nice with selectors * update display for host to ensure we can layout the element properly margin, etc... * update latest FAST packages * leverage ValuesOf type helper
radium-v
pushed a commit
that referenced
this pull request
May 2, 2024
* add text as a new component * update main.js to .cjs format for esm due to require syntax * update style rules to align with FUI react v9, export text options, update stories to leverage object.keys of options * export text, update node resolution to 16 and fix object.keys for size to object.values * prettier style files, may need to revert if it does not play nice with selectors * update display for host to ensure we can layout the element properly margin, etc... * update latest FAST packages * leverage ValuesOf type helper
radium-v
pushed a commit
that referenced
this pull request
May 2, 2024
* add text as a new component * update main.js to .cjs format for esm due to require syntax * update style rules to align with FUI react v9, export text options, update stories to leverage object.keys of options * export text, update node resolution to 16 and fix object.keys for size to object.values * prettier style files, may need to revert if it does not play nice with selectors * update display for host to ensure we can layout the element properly margin, etc... * update latest FAST packages * leverage ValuesOf type helper
radium-v
pushed a commit
that referenced
this pull request
May 2, 2024
* add text as a new component * update main.js to .cjs format for esm due to require syntax * update style rules to align with FUI react v9, export text options, update stories to leverage object.keys of options * export text, update node resolution to 16 and fix object.keys for size to object.values * prettier style files, may need to revert if it does not play nice with selectors * update display for host to ensure we can layout the element properly margin, etc... * update latest FAST packages * leverage ValuesOf type helper
radium-v
pushed a commit
that referenced
this pull request
May 3, 2024
* add text as a new component * update main.js to .cjs format for esm due to require syntax * update style rules to align with FUI react v9, export text options, update stories to leverage object.keys of options * export text, update node resolution to 16 and fix object.keys for size to object.values * prettier style files, may need to revert if it does not play nice with selectors * update display for host to ensure we can layout the element properly margin, etc... * update latest FAST packages * leverage ValuesOf type helper
radium-v
pushed a commit
that referenced
this pull request
May 6, 2024
* add text as a new component * update main.js to .cjs format for esm due to require syntax * update style rules to align with FUI react v9, export text options, update stories to leverage object.keys of options * export text, update node resolution to 16 and fix object.keys for size to object.values * prettier style files, may need to revert if it does not play nice with selectors * update display for host to ensure we can layout the element properly margin, etc... * update latest FAST packages * leverage ValuesOf type helper
radium-v
pushed a commit
that referenced
this pull request
May 6, 2024
* add text as a new component * update main.js to .cjs format for esm due to require syntax * update style rules to align with FUI react v9, export text options, update stories to leverage object.keys of options * export text, update node resolution to 16 and fix object.keys for size to object.values * prettier style files, may need to revert if it does not play nice with selectors * update display for host to ensure we can layout the element properly margin, etc... * update latest FAST packages * leverage ValuesOf type helper
radium-v
pushed a commit
that referenced
this pull request
May 8, 2024
* add text as a new component * update main.js to .cjs format for esm due to require syntax * update style rules to align with FUI react v9, export text options, update stories to leverage object.keys of options * export text, update node resolution to 16 and fix object.keys for size to object.values * prettier style files, may need to revert if it does not play nice with selectors * update display for host to ensure we can layout the element properly margin, etc... * update latest FAST packages * leverage ValuesOf type helper
radium-v
pushed a commit
that referenced
this pull request
May 10, 2024
* add text as a new component * update main.js to .cjs format for esm due to require syntax * update style rules to align with FUI react v9, export text options, update stories to leverage object.keys of options * export text, update node resolution to 16 and fix object.keys for size to object.values * prettier style files, may need to revert if it does not play nice with selectors * update display for host to ensure we can layout the element properly margin, etc... * update latest FAST packages * leverage ValuesOf type helper
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New Behavior
This PR does two things:
Adds Text as a new component
Updates the package to support ESM and get storybook building, etc.
While I would have rather done separate PR's for this, and am happy to if we need to, in order to get storybook up and running on ESM I needed to have something to build :). The first commit contains most all the component code. The second is primarily the ESM updates within the repo (pathing, storybook webpack changes, dep updates locally for typescript, tslib, and ts-loader.
Related Issue(s)