Skip to content

Component: vf-dropdown#1679

Merged
nitinja merged 7 commits intodevelopfrom
components/vf-dropdown
Sep 28, 2021
Merged

Component: vf-dropdown#1679
nitinja merged 7 commits intodevelopfrom
components/vf-dropdown

Conversation

@nitinja
Copy link
Copy Markdown
Contributor

@nitinja nitinja commented Sep 27, 2021

Initial commit for vf-dropdown, issue #1299

@nitinja nitinja requested a review from khawkins98 September 27, 2021 13:40
@netlify
Copy link
Copy Markdown

netlify bot commented Sep 27, 2021

✔️ Deploy Preview for ecstatic-noether-150ab4 ready!

🔨 Explore the source changes: 1b3a79a

🔍 Inspect the deploy log: https://app.netlify.com/sites/ecstatic-noether-150ab4/deploys/61531dc61aa11c0007fb8bc6

😎 Browse the preview: https://deploy-preview-1679--ecstatic-noether-150ab4.netlify.app

@nitinja
Copy link
Copy Markdown
Contributor Author

nitinja commented Sep 27, 2021

Note that I added z-index metadata for variables, but couldn't get it working, so hardcoded values as of now. @khawkins98 do we have to run any command to compile these yaml values into scc/css?

@nitinja nitinja requested a review from cindyebi September 28, 2021 08:43
@khawkins98
Copy link
Copy Markdown
Contributor

Note that I added z-index metadata for variables, but couldn't get it working, so hardcoded values as of now. @khawkins98 do we have to run any command to compile these yaml values into scc/css?

Yeah, to get the design tokens to update you need to:

  1. cd components/vf-design-tokens
  2. yarn install
  3. gulp vf-tokens

That will compile them and make them available. The updated files need to be committed.

Looking around the code I don't think these steps were ever documented — I'll add that to my to-do list.

About should be one short sentence.
Comment thread components/vf-dropdown/vf-dropdown.njk Outdated
Comment thread components/vf-dropdown/vf-dropdown.scss Outdated
Comment thread components/vf-dropdown/vf-dropdown.scss Outdated
Comment thread components/vf-dropdown/vf-dropdown.js Outdated
Comment thread components/vf-dropdown/vf-dropdown.js Outdated
Comment thread components/vf-dropdown/vf-dropdown.jsx Outdated
@cindyebi
Copy link
Copy Markdown

@nitinja is it possible to see some screenshots/visuals for this?

Comment thread components/vf-dropdown/vf-dropdown.scss Outdated
@nitinja
Copy link
Copy Markdown
Contributor Author

nitinja commented Sep 28, 2021

HI @khawkins98 fixed review comments except one

@khawkins98
Copy link
Copy Markdown
Contributor

I've also compiled the tokens and pushed those.

@nitinja nitinja merged commit 4db92db into develop Sep 28, 2021
@nitinja nitinja deleted the components/vf-dropdown branch September 28, 2021 13:52
khawkins98 added a commit that referenced this pull request Sep 29, 2021
Following work in #1682 and #1679 to introduce the new alpha components this cleans up a pair of rough edges.

- css custom prop usage
- ie11 fallback
- njk typo
- njk whitespace control
- add aria progress  bar role
  - maybe more work to do there in the future https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_progressbar_role
- add context conditionals
nitinja pushed a commit that referenced this pull request Sep 29, 2021
* chore: drop-down and progress component cleanup

Following work in #1682 and #1679 to introduce the new alpha components this cleans up a pair of rough edges.

- css custom prop usage
- ie11 fallback
- njk typo
- njk whitespace control
- add aria progress  bar role
  - maybe more work to do there in the future https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_progressbar_role
- add context conditionals

* Update vf-progress-indicator.jsx

* CSS linting and nesting

* More linting

* Use z-index tokens, context

* Small typo fixes

* Remove unused event handler
@nitinja
Copy link
Copy Markdown
Contributor Author

nitinja commented Sep 29, 2021

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants