feat: toggle follow fiori3 #492

JKMarkowski · 2019-12-05T18:03:08Z

Related Issue

Closes #456
Closes #535

Description

There are is new toggle, which follows fiori3 specs

Breaking changes

Completely new structure of toggle in every mode. Added classes:
fd-toggle__text - To put some text (max 3 chars) inside toggle.
fd-toggle__wrapper, fd-toggle__track.
Removed size classes (--xs, --s, --m, --l, --xl)

Screenshots

NOTE: If you've made any style changes, please provide appropriate screenshots (before and after) to help reviewers.

To see examples of which screenshots to include, go to Screenshot Examples.

Before:

After:

netlify · 2019-12-05T18:05:59Z

Deploy preview for fundamental-styles ready!

Built with commit f43b06b

https://deploy-preview-492--fundamental-styles.netlify.com

src/toggle.scss

stefanoScalzo · 2019-12-18T19:19:00Z

src/form-label.scss

@@ -84,5 +84,8 @@ $block: #{$fd-namespace}-form-label;
    overflow: initial;
    margin-top: 5px;
    margin-bottom: 5px;
+    display: flex;


why is this needed?

InnaAtanasova

Since we are introducing Breaking changes for this component I think it's a good idea to also rename the classes from toggle to switch

InnaAtanasova · 2019-12-18T20:06:38Z

docs/pages/components/toggle.md


 {% capture default %}
 <div class="fd-form-group">
    <div class="fd-form-item">
-        <label class="fd-form-label" for="y21YO391">Live:</label>
+        <label class="fd-form-label">Toggle with label</label>
        <label class="fd-form-label fd-form-label--toggle">


This will make the toggle component dependent on fd-form-label. Can you create a new class and if you depend a lot on fd-form-label maybe create a mixin and include it here.

InnaAtanasova · 2019-12-18T20:09:38Z

docs/pages/components/toggle.md

+                <input class="fd-toggle__input" type="checkbox" name="" value="" id="y21YO3911">
+                <div class="fd-toggle__wrapper">
+                    <div class="fd-toggle__inner">
+                        <span class="fd-toggle__label fd-toggle__label--on">on</span>


fd-switch__text

I wanted to use fd-toggle__label not only for text, it's also for icons.

I will use __text, because __label is reserved for labels

InnaAtanasova · 2019-12-18T20:10:02Z

docs/pages/components/toggle.md

+                <div class="fd-toggle__wrapper">
+                    <div class="fd-toggle__inner">
+                        <span class="fd-toggle__label fd-toggle__label--on">on</span>
+                        <span class="fd-toggle__switch" role="presentation"></span>


fd-switch__handle

InnaAtanasova · 2019-12-18T20:11:14Z

docs/pages/components/toggle.md

-                <span class="fd-toggle__switch" role="presentation"></span>
+                <input class="fd-toggle__input" type="checkbox" name="" value="" id="y21YO3911">
+                <div class="fd-toggle__wrapper">
+                    <div class="fd-toggle__inner">


fd-switch__track

InnaAtanasova · 2019-12-18T20:21:56Z

src/toggle.scss

+    background-color: $fd-toggle-inactive-switch-background;
+    width: $fd-toggle-switch-size;
+    height: $fd-toggle-switch-size;
+    border-radius: $fd-toggle-switch-border-radius;


missing background-clip: padding-box;

InnaAtanasova · 2019-12-18T20:31:11Z

src/toggle.scss

+  $fd-toggle-container-padding-y: 0.375rem;
+  $fd-toggle-size-x: 3.75rem;
+  $fd-toggle-size-y: 1.25rem;
+  $fd-toggle-switch-size: 1.875rem;


Isn't it 2rem?

It is, but actually it's counted with (2* 0.0625rem) border, so it gives 2rem.

JKMarkowski · 2020-01-08T13:41:50Z

All of the comments are solved now. It's ready to second review.

stefanoScalzo · 2020-01-09T15:31:23Z

docs/pages/components/toggle.md

-            <span class="fd-toggle">
-                <input class="fd-toggle__input" type="checkbox" name="" value="" id="y21YO391">
-                <span class="fd-toggle__switch" role="presentation"></span>
+        <label class="fd-form-label">Toggle with label</label>


shouldn't you only use the switch label?

Labels are not stricly connected to switch. So I don't think so

InnaAtanasova · 2020-01-14T20:23:38Z

The elements are not focusable and the keyboard support is missing
There's bleeding of styles - check the un-normalize in the Playground

You need to reset the fd-switch__wrapper, fd-switch__track, and fd-switch__handle.
You will get something like:

InnaAtanasova · 2020-01-15T18:48:12Z

The tests are failing because the "handle" moved a bit (prob because of the resets). I think it's related to the left: calc(1.8125rem - 100%); for fd-switch__track.

bcullman · 2020-01-16T18:13:53Z

can we see a before and after screen shots on the PR description?

JKMarkowski · 2020-01-17T14:40:19Z

@bcullman I noticed your comments too late, after merging this commit. But it has been tested and compared to fiori3 specs and everything works correct.

bcullman · 2020-01-17T16:02:01Z

@thanks @JKMarkowski looks great! Thats been wrong for a long time. Glad thats been fixed.

I did note one other oddity on the alert - when in RTL mode, the close button is off by pixel from the edge. See

vs rtl

JKMarkowski requested review from jbadan, InnaAtanasova, stefanoScalzo and droshev December 5, 2019 18:03

JKMarkowski added the Fiori 3 refactoring match Fiori 3 requirements label Dec 5, 2019

stefanoScalzo suggested changes Dec 5, 2019

View reviewed changes

src/toggle.scss Show resolved Hide resolved

src/toggle.scss Outdated Show resolved Hide resolved

src/toggle.scss Outdated Show resolved Hide resolved

JKMarkowski requested a review from stefanoScalzo December 6, 2019 13:02

salarenko mentioned this pull request Dec 17, 2019

Toggle switch handle trimmed, no switch animation #535

Closed

stefanoScalzo reviewed Dec 18, 2019

View reviewed changes

InnaAtanasova requested changes Dec 18, 2019

View reviewed changes

InnaAtanasova reviewed Dec 18, 2019

View reviewed changes

InnaAtanasova added the 0.5.0 label Dec 20, 2019

JKMarkowski added 8 commits January 6, 2020 14:03

Adjust Toggle to fiori3

5009fc7

Add tests screens reference

f939456

Fix RTL mode on IE

7e19e12

Add margins to toggle labels

27f7639

Update tests

34929b5

Change docs

7ecf825

PR naming changes

22db22d

Change naming, make label self-contained, change test naming

f741c54

JKMarkowski force-pushed the fix/456-toggle branch from 65e1b2f to f741c54 Compare January 6, 2020 13:56

Update Reference Images

cb3b331

JKMarkowski requested review from InnaAtanasova and stefanoScalzo January 6, 2020 15:42

InnaAtanasova added this to In progress in Development via automation Jan 6, 2020

InnaAtanasova added this to the Sprint 28 - Sydney milestone Jan 6, 2020

InnaAtanasova moved this from In progress to Review in progress in Development Jan 6, 2020

stefanoScalzo reviewed Jan 9, 2020

View reviewed changes

Add focus/keyboard support, reset elements

7ad509d

Add content-box to components

f43b06b

InnaAtanasova approved these changes Jan 16, 2020

View reviewed changes

stefanoScalzo self-requested a review January 16, 2020 17:48

Development automation moved this from PRs Review in progress to PRs Reviewer approved Jan 16, 2020

stefanoScalzo approved these changes Jan 16, 2020

View reviewed changes

JKMarkowski merged commit a4c40a3 into master Jan 17, 2020

Development automation moved this from PRs Reviewer approved to Done Jan 17, 2020

JKMarkowski deleted the fix/456-toggle branch January 17, 2020 14:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: toggle follow fiori3 #492

feat: toggle follow fiori3 #492

JKMarkowski commented Dec 5, 2019 •

edited

netlify bot commented Dec 5, 2019 •

edited

stefanoScalzo Dec 18, 2019

InnaAtanasova left a comment

InnaAtanasova Dec 18, 2019

InnaAtanasova Dec 18, 2019

JKMarkowski Dec 20, 2019

JKMarkowski Jan 6, 2020

InnaAtanasova Dec 18, 2019

InnaAtanasova Dec 18, 2019

InnaAtanasova Dec 18, 2019

InnaAtanasova Dec 18, 2019

JKMarkowski Dec 20, 2019

JKMarkowski commented Jan 8, 2020

stefanoScalzo Jan 9, 2020

JKMarkowski Jan 10, 2020 •

edited

InnaAtanasova commented Jan 14, 2020 •

edited

InnaAtanasova commented Jan 15, 2020

bcullman commented Jan 16, 2020

JKMarkowski commented Jan 17, 2020

bcullman commented Jan 17, 2020

feat: toggle follow fiori3 #492

feat: toggle follow fiori3 #492

Conversation

JKMarkowski commented Dec 5, 2019 • edited

Related Issue

Description

Breaking changes

Screenshots

Before:

After:

netlify bot commented Dec 5, 2019 • edited

Choose a reason for hiding this comment

InnaAtanasova left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JKMarkowski commented Jan 8, 2020

Choose a reason for hiding this comment

JKMarkowski Jan 10, 2020 • edited

Choose a reason for hiding this comment

InnaAtanasova commented Jan 14, 2020 • edited

InnaAtanasova commented Jan 15, 2020

bcullman commented Jan 16, 2020

JKMarkowski commented Jan 17, 2020

bcullman commented Jan 17, 2020

JKMarkowski commented Dec 5, 2019 •

edited

netlify bot commented Dec 5, 2019 •

edited

JKMarkowski Jan 10, 2020 •

edited

InnaAtanasova commented Jan 14, 2020 •

edited