Skip to content

Add semantic slots, starting with text fields#1568

Merged
phkuo merged 18 commits intomicrosoft:masterfrom
phkuo:phkuo/semanticInputs
May 9, 2017
Merged

Add semantic slots, starting with text fields#1568
phkuo merged 18 commits intomicrosoft:masterfrom
phkuo:phkuo/semanticInputs

Conversation

@phkuo
Copy link
Copy Markdown
Contributor

@phkuo phkuo commented Apr 21, 2017

Pull request checklist

  • Addresses an existing issue: #0000
  • Include a change request file if publishing
  • New feature, bugfix, or enhancement
    • Includes tests
  • Documentation update

Description of changes

Lay some groundwork for converting to semantic slots, starting with input text fields.

Focus areas to test

(optional)

@micahgodbolt
Copy link
Copy Markdown
Member

Looks fine to me (assuming no visual regressions). I've seen this approach before from @phkuo and it closely mirrors how I'd do it with external Theo tokens. I'm mostly curious how this would look using Glamor, though I don't feel anything here would make converting to Glamor harder.

Maybe the bigger question is how close are we to converting to Glamor (pretty close I think), and should this work just be done in coordination with that glamor conversion.

@phkuo phkuo force-pushed the phkuo/semanticInputs branch from db1a99d to 2c36116 Compare April 21, 2017 21:51
@micahgodbolt
Copy link
Copy Markdown
Member

@dzearing how are you feeling about this approach and what it'd look like with Glamor? I don't see any problem creating something similar, but wanted to get your feedback before we merge.

And @phkuo, just an FYI that you have some conflicts to resolve.

@phkuo phkuo force-pushed the phkuo/semanticInputs branch from 312b248 to b90c9db Compare April 27, 2017 18:06
// --------------------------------------------------
// ChoiceField semantic slots

$radioButton-background-color: $controlBackgroundColor;
Copy link
Copy Markdown
Member

@dzearing dzearing May 2, 2017

Choose a reason for hiding this comment

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

is a radio button any different from a checkbox?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately yes. For example, in the selected state, it uses a blue background and a white checkmark, whereas radio buttons have a white background and a blue dot.

@phkuo
Copy link
Copy Markdown
Contributor Author

phkuo commented May 2, 2017

Doing design review before completing this PR.
@betrue-final-final

@phkuo
Copy link
Copy Markdown
Contributor Author

phkuo commented May 5, 2017

@dzearing @micahgodbolt Could I get this reviewed and signed off please?

@micahgodbolt
Copy link
Copy Markdown
Member

@phkuo a few conflicts to resolve in textfield

@phkuo phkuo force-pushed the phkuo/semanticInputs branch from 587039b to a2c4e09 Compare May 8, 2017 22:33
@phkuo phkuo merged commit bb7474c into microsoft:master May 9, 2017
@phkuo phkuo deleted the phkuo/semanticInputs branch June 7, 2018 21:55
@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants