-
Notifications
You must be signed in to change notification settings - Fork 233
docs: textfield-based component analysis docs #5741
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
base: 2nd-gen-component-analysis
Are you sure you want to change the base?
docs: textfield-based component analysis docs #5741
Conversation
|
830facd
to
2c217ea
Compare
📚 Branch Preview🔍 Visual Regression Test ResultsWhen a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
Deployed to Azure Blob Storage: If the changes are expected, update the |
Tachometer resultsCurrently, no packages are changed by this PR... |
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.
I feel like I've been holding onto a bunch of comments so...here come a flood! I have to go through the text field file, but that one was just so long and I didn't want to hold these up.
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.
Leaving the remainder for textfield.
|
||
- **Label position**: This is controlled by field label in SWC, but controlled by textfield in CSS | ||
- **Character count**: SWC doesn't currently support a visible character count that would track the number of characters within the text field | ||
- **Keyboard focus state**: CSS has slightly different styling for focused vs. keyboard-focused state (keyboard focus shows the blue outline, mouse focus does not), this distinction is not visible in the current SWC implementation |
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.
oh great callout here about the differences between focused and keyboard-focused!
|
||
### Common issues to watch for | ||
|
||
- **Branch confusion**: AI may analyze wrong branches or mix up repositories |
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.
I love AI writing and summarizing all the things that AI might not be able to do here 😆
a509ac1
to
268b194
Compare
<details> | ||
<summary>Diff: Legacy (CSS main) → Spectrum 2 (CSS spectrum-two)</summary> | ||
|
||
```diff |
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.
Happy to have any feedback on this numberfield diff... it's really unpleasant to look at, I think, because just about every line changes due to class name changes.
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.
Could we preface the diff with something to the affect of: "Most of the diff below is due to the change in classname from .spectrum-Stepper
to .spectrum-NumberField
"?
Regarding the number field diff also- we could foreseeably add those --top
and --bottom
modifier classes to the legacy infield buttons, as well. They're in the legacy DOM example, just not in this portion with the markup diff. And speaking of infield buttons- the changes to infield button are also a big reason this diff looks so severe.
Just some thoughts.
migration-roadmap/search.md
Outdated
| | `action` | Missing from CSS | | ||
| | `method` | Missing from CSS | | ||
| | `holdValueOnEscape` | Missing from CSS | | ||
| | `negative-help-text` slot | Missing from CSS | |
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.
"Missing from CSS" doesn't feel super valuable for our purposes, taking this out if no one is opposed.
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.
I know I left a TON of comments, and everything is looking great! Thanks for all of your hard work on this! I only found a couple of really small things this time around! 🎉
|
||
- **CSS selectors**: All selectors from `metadata.json` in the spectrum-css `spectrum-two` branch | ||
- **Passthroughs**: CSS passthrough properties | ||
- **Modifiers**: CSS modifier classes |
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.
I know the mods stuff is up in the air, but we've been listing the modifier custom properties as opposed to classes.
**Inherited from SizedMixin:** | ||
|
||
- `size` - Size of the color field (s, m, l, xl) | ||
|
||
**Inherited from Focusable:** |
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.
I love the reorganization of all of this information you and Cassondra have done 😍
+ Label Text | ||
+ </label> | ||
<span class="spectrum-Textfield-characterCount">Character Count</span> | ||
- <div class="spectrum-Icon spectrum-Icon--sizeM spectrum-Textfield-validationIcon"> |
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.
If we're trying to be accurate, we should probably remove this div since it's not in the legacy markup.
I do love the .spectrum-Icon-spectrum
class though 😆
<details> | ||
<summary>Diff: Legacy (CSS main) → Spectrum 2 (CSS spectrum-two)</summary> | ||
|
||
```diff |
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.
Could we preface the diff with something to the affect of: "Most of the diff below is due to the change in classname from .spectrum-Stepper
to .spectrum-NumberField
"?
Regarding the number field diff also- we could foreseeably add those --top
and --bottom
modifier classes to the legacy infield buttons, as well. They're in the legacy DOM example, just not in this portion with the markup diff. And speaking of infield buttons- the changes to infield button are also a big reason this diff looks so severe.
Just some thoughts.
@@ -0,0 +1,174 @@ | |||
# Spectrum 2 migration roadmap | |||
|
|||
This directory contains comprehensive migration documentation for 2nd generation Spectrum Web Components based on the implementation of Spectrum 2 components that were previously migrated in [Spectrum CSS](https://github.com/adobe/spectrum-css/tree/spectrum-two). It helps engineers understand what needs to be implemented, updated, or aligned between the two systems to guide development of 2nd generation web 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.
Love this! Fantastic summary of the work being done here. 🕺
|
||
- **CSS selectors**: All selectors from `metadata.json` in the spectrum-css `spectrum-two` branch | ||
- **Passthroughs**: CSS passthrough properties | ||
- **Modifiers**: CSS modifier classes |
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.
Might be good to also flag that these are deprecated in S2 (so this information is mostly for migration guide purposes).
- **CSS selectors**: All selectors from `metadata.json` in the spectrum-css `spectrum-two` branch | ||
- **Passthroughs**: CSS passthrough properties | ||
- **Modifiers**: CSS modifier classes | ||
- **SWC attributes**: Properties with `@property` decorators in TypeScript |
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.
Tiny nit, these can also be handled using getters and setters (@property
is an alias for a simple getter and setter pattern).
- **Passthroughs**: CSS passthrough properties | ||
- **Modifiers**: CSS modifier classes | ||
- **SWC attributes**: Properties with `@property` decorators in TypeScript | ||
- **SWC slots**: Slot patterns from render methods |
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.
Should we list out nested components too? Ones that exist in the component's shadow DOM? That might give us a good sense of in what order we should be migrating components.
|
||
- CSS selectors (collapsible) | ||
- Passthroughs (collapsible) | ||
- Modifiers (collapsible) |
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.
- Modifiers (collapsible) | |
- Modifiers **deprecated** (collapsible) |
|
||
### CSS | ||
|
||
- CSS selectors (collapsible) |
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.
Should we highlight the updated approach we've been doing of grouping selectors into elements, states, and variants?
- Spectrum 2 CSS spectrum-two branch (collapsible) | ||
- Diff: Legacy → Spectrum 2, based on changes made to template in CSS migration work (collapsible, if changes exist) | ||
|
||
### CSS => SWC mapping |
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.
🕺 Love this table; I think it's going to be really critical as we being our component migration efforts in earnest.
|
||
### CSS => SWC implementation gaps | ||
|
||
### CSS Spectrum 2 changes |
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.
Should we add a note somewhere that some of this information might be summarized in the component's changelog in the spectrum-two
branch?
|
||
## How to generate documentation | ||
|
||
### Prerequisites |
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.
LOVE! Thanks for adding this.
- `spectrum-css` (primarily `spectrum-two` branch, with comparisons to `main`) | ||
- `spectrum-web-components` (`main` branch) | ||
|
||
### Using the cursor prompt |
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.
Let's add a suggestion here for what models to use. This type of work is better handled by the slower but more advanced thinking models like Claude or gpt-5
Description
Creates AI-generated migration documentation to analyze component differences to guide SWC migration to S2, with human vetting. The documentation serves as a bridge between the migrated Spectrum 2 CSS work and the corresponding web components, in order to help engineers understand what needs to be implemented, updated, or aligned between the two systems to guide the development of 2nd generation web components.
This batch is for the barebones components: Textfield/Textarea, Number field, Search, and Color field (which does not exist in CSS or S2 design spec)
Motivation and context
Related issue(s)
SWC-1208
Screenshots (if appropriate)
Author's checklist
I have added automated tests to cover my changes.I have included a well-written changeset if my change needs to be published.Reviewer's checklist
patch
,minor
, ormajor
featuresManual review test cases
Documentation Quality
Cross-Reference Accuracy
metadata.json
filesDevice review