Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: remainder of design system knobs #3171

Merged
merged 3 commits into from
May 22, 2020
Merged

feat: remainder of design system knobs #3171

merged 3 commits into from
May 22, 2020

Conversation

eljefe223
Copy link
Contributor

@eljefe223 eljefe223 commented May 20, 2020 •
edited
Loading

Description

This change makes styling improvements and hooks up the remainder of the design system knobs except the hue and saturation sliders.

UPDATE Added hue and saturation sliders

Screen Shot 2020-05-20 at 4 53 33 PM

Screen Shot 2020-05-21 at 12 33 15 PM

Screen Shot 2020-05-20 at 4 53 22 PM

Screen Shot 2020-05-20 at 4 53 16 PM

Issue type checklist

  • Chore: A change that does not impact distributed packages.
  • Bug fix: A change that fixes an issue, link to the issue above.
  • New feature: A change that adds functionality.

Is this a breaking change?

  • This change causes current functionality to break.

Process & policy checklist

  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

@eljefe223 eljefe223 requested a review from janechu as a code owner May 20, 2020 23:58
@eljefe223 eljefe223 self-assigned this May 20, 2020
@awentzel awentzel changed the title feat: hook up design system knobs feat: update design system with greater ability to adjust settings May 21, 2020
@awentzel awentzel changed the title feat: update design system with greater ability to adjust settings feat: update design system to finalize styling adjustments May 21, 2020
@eljefe223 eljefe223 requested review from anewsam and SamanthaAO and removed request for anewsam May 21, 2020 19:39
@khamudom
Copy link
Contributor

khamudom commented May 21, 2020 •
edited
Loading

Not sure if it was just missed or something you were going to tackle later. Should it say Dark Mode?
Screen Shot 2020-05-21 at 12 41 24 PM

khamudom
khamudom approved these changes May 21, 2020
View reviewed changes
Copy link
Contributor

@khamudom khamudom left a comment

Choose a reason for hiding this comment

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

I have smoked the changes. The only major issue noticed is the sliders and I believe that is being addressed now in another PR.

@eljefe223
Copy link
Contributor Author

Not sure if it was just missed or something you were going to tackle later. Should it say Dark Mode?
Screen Shot 2020-05-21 at 12 41 24 PM

It does say Dark mode, I updated that in the latest commit

@eljefe223 eljefe223 force-pushed the users/jes/more-styling branch from 7deef86 to 237cb21 Compare May 21, 2020 20:38
@khamudom
Copy link
Contributor

Not sure if it was just missed or something you were going to tackle later. Should it say Dark Mode?
Screen Shot 2020-05-21 at 12 41 24 PM

It does say Dark mode, I updated that in the latest commit

I must have been smoking it before and while you pushed an update. All is good!

@eljefe223 eljefe223 changed the title feat: update design system to finalize styling adjustments feat: remainder of design system knobs May 21, 2020
@SamanthaAO
Copy link
Contributor

Is the slider bug also responsible for the label of 15px being mispositioned on base height multiplier?

@eljefe223
Copy link
Contributor Author

Is the slider bug also responsible for the label of 15px being mispositioned on base height multiplier?

This is fixed yes, we just need to rebase with master

@SamanthaAO
Copy link
Contributor

SamanthaAO commented May 21, 2020 •
edited
Loading

When I use the base horizontal spacing multiplier slider it does not seem the change anything in the previews layout.

@eljefe223
Copy link
Contributor Author

When I use the base horizontal spacing multiplier slider it does not seem the change anything in the previews layout.

That is because we don't actually bind css values to it currently. We may want to revisit keeping the slider if it does not add value. But we typically using that for left and right margin so we probably will do some work in fast-components to start replacing an hardcoded values.

@eljefe223 eljefe223 merged commit 1c54d92 into feature/fast-frame May 22, 2020
@eljefe223 eljefe223 deleted the users/jes/more-styling branch May 22, 2020 18:05
chrisdholt pushed a commit that referenced this pull request Jun 4, 2020
@eljefe223 @chrisdholt
feat: remainder of design system knobs (#3171)
1c86926 
* feat: hook up design sytem controls

* added hue and sat sliders

* typed signatures
chrisdholt pushed a commit that referenced this pull request Jun 11, 2020
@eljefe223 @chrisdholt
feat: remainder of design system knobs (#3171)
0bb2414 
* feat: hook up design sytem controls

* added hue and sat sliders

* typed signatures
chrisdholt pushed a commit that referenced this pull request Jun 16, 2020
@eljefe223 @chrisdholt
feat: remainder of design system knobs (#3171)
8ba91a1 
* feat: hook up design sytem controls

* added hue and sat sliders

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

@SamanthaAO SamanthaAO SamanthaAO approved these changes

@khamudom khamudom khamudom approved these changes

@nicholasrice nicholasrice nicholasrice approved these changes

@radium-v radium-v Awaiting requested review from radium-v

@chrisdholt chrisdholt Awaiting requested review from chrisdholt

@janechu janechu Awaiting requested review from janechu

Assignees

@eljefe223 eljefe223

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@eljefe223 @khamudom @SamanthaAO @nicholasrice