fix: Step Input #986

JKMarkowski · 2020-05-08T09:33:54Z

Related Issue

fixes: #779

Description

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 · 2020-05-08T09:37:15Z

Deploy preview for fundamental-styles ready!

Built with commit 456ce56

https://deploy-preview-986--fundamental-styles.netlify.app

droshev · 2020-05-08T18:10:35Z

Can you create a storybook instead of njk visual tests?

Vanessa-Cusmich · 2020-05-11T02:05:48Z

docs/pages/components/step-input.md

+folder: components
+---
+
+The Step Input component is an opinionated composition of the Input, Button components.


Can we use the description from the Fiori Guidelines? I think it sounds better. I also included the usage guidelines in case you want to add them.

The step input control allows the user to change the input values in predefined increments (steps).

Usage
Use the step input if:
The user needs to adjust amounts, quantities, or other values quickly.
The user needs to adjust values for a specific step (for example, in a shopping cart).
Do not use the step input if:
The user needs to enter a static number (for example, postal code, phone number, or ID). In this case, use the regular input field control instead.
You want to display a value that rarely needs to be adjusted and does not pertain to a particular step. In this case, use the regular input field control instead.
You want the user to enter dates and times. In this case, use the date picker, date range selection, time picker, or date/time picker instead.

Vanessa-Cusmich · 2020-05-11T02:12:33Z

docs/pages/components/step-input.md

+
+
+## Step Input with Cozy
+Cozy Mode is the default one, it should be used in mobile and tablet devices.


On smartphones and tablets, the step input is shown in cozy mode (default).

Vanessa-Cusmich · 2020-05-11T02:15:11Z

docs/pages/components/step-input.md

+
+
+## Compact Step Input
+Step Input can be used in compact mode, which is done to be used in desktop, or larger screen devices. 


The Step Input should be used in compact mode when using a desktop of devices with large screens. It can be achieved by adding the --compact modifier to the main element as well as the button and input elements.

Vanessa-Cusmich · 2020-05-11T02:16:59Z

docs/pages/components/step-input.md

+{% include display-component.html component=step-input-compact %}
+
+## Step Input with Different states
+Step Input Component also supports semantic states, same as for every form. They can be achieved by adding


The Step Input component also supports semantic states as does every form. The semantic states can be customized by adding the is-error | is-success | is-warning | or is-information into the fd-step-input element.

JKMarkowski · 2020-05-11T08:33:16Z

@droshev

Can you create a storybook instead of njk visual tests?

For now both are added

salarenko · 2020-05-11T09:25:49Z

src/step-input.scss

+  $fd-step-input-compact-button-width: 1.5rem;
+  $fd-step-input-description-padding: 0.5rem;
+
+  @mixin iconSize($size) {


Is it worth to create a mixin if its used only once?

salarenko · 2020-05-11T09:27:02Z

src/step-input.scss

+  border-color: var(--sapField_BorderColor);
+  border-radius: var(--sapField_BorderCornerRadius);
+  background-color: var(--sapField_Background);
+  height: calc(#{$fd-step-input-cozy-height} + 2 * var(--sapField_BorderWidth));


I don't know if this is correct because there is no exact value in spec, but our standard input height is 36px and step-input height is 38px.

You are right, there should be 36px with borders included (34px + 2px)

salarenko · 2020-05-11T09:27:35Z

src/step-input.scss

+    @include fd-focus() {
+      outline: none;
+
+      &,


Shouldn't you use @include fd-hover() here?

salarenko · 2020-05-11T09:31:17Z

src/step-input.scss

+  }
+
+  /** Width is different due to different border width  */
+  &.is-information,


Missing success state

success has 1px from mixin

salarenko · 2020-05-11T09:31:44Z

src/step-input.scss

+  &.is-error,
+  &.is-warning {
+    .#{$block}__button.#{$fd-namespace}-button {
+      width: calc(#{$fd-step-input-cozy-button-width} - 0.125rem);


We should move 0.125rem to a variable

JKMarkowski · 2020-05-11T12:00:39Z

All changes are addressed, it's ready to be reviewed again

Vanessa-Cusmich

LGTM

mikerodonnell89

can we just have --no-number-spinner be default behavior for our number type inputs so no extra class is needed?

JKMarkowski · 2020-05-14T15:53:05Z

can we just have --no-number-spinner be default behavior for our number type inputs so no extra class is needed?

We decided to have it in other PR

prsdthkr · 2020-05-14T16:47:21Z

It think it is because of the Fiori 3 spec, but the focus ring is not visible on the individual step up and step down buttons as seen in the patterns example. I can see this becoming an accessibility issue.

Patterns example shows focus ring on individual buttons:

InnaAtanasova

Looks great!

prsdthkr · 2020-05-14T22:24:41Z

It think it is because of the Fiori 3 spec, but the focus ring is not visible on the individual step up and step down buttons as seen in the patterns example. I can see this becoming an accessibility issue.

Correction: It is okay that the focus ring is not appearing on the step buttons. This is in line with the <input type="number"> implementation as seen here: https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/step.

But we should also make sure that the buttons inside the input are not tab-able. Currently there is an invisible tab stop. As seen in the above GIF.

JKMarkowski · 2020-05-15T07:23:15Z

@prsdthkr Thanks for pointing it, I will add tabindex: -1 to all buttons, to make them not tabable

JKMarkowski added the 0.10.0 label May 8, 2020

JKMarkowski added this to the Sprint 36 - Plovdiv milestone May 8, 2020

JKMarkowski requested a review from a team May 8, 2020 09:33

Vanessa-Cusmich suggested changes May 11, 2020

View reviewed changes

JKMarkowski force-pushed the feat/step-input2 branch from fb9f54d to b9fbf45 Compare May 11, 2020 08:35

salarenko suggested changes May 11, 2020

View reviewed changes

JKMarkowski requested review from salarenko and Vanessa-Cusmich May 11, 2020 12:00

Vanessa-Cusmich approved these changes May 11, 2020

View reviewed changes

Vanessa-Cusmich modified the milestones: Sprint 36 - Plovdiv, Sprint 37 - Shenzhen May 11, 2020

InnaAtanasova assigned JKMarkowski May 11, 2020

mikerodonnell89 self-requested a review May 12, 2020 16:01

mikerodonnell89 requested changes May 12, 2020

View reviewed changes

JKMarkowski requested a review from mikerodonnell89 May 13, 2020 15:16

mikerodonnell89 approved these changes May 14, 2020

View reviewed changes

InnaAtanasova approved these changes May 14, 2020

View reviewed changes

manu-kr and others added 4 commits May 15, 2020 09:28

feat(step-input):add step input component

d272bef

feat(step-input):renaming of css class name step input

53e76a7

Fix(step-input): Redesign step-input using input group and button

bfe81e5

update visual tests

d2dce93

JKMarkowski added 10 commits May 15, 2020 09:28

fix: Refactor step input component, move unit-description to labels

84eb641

Update ref images

988565b

save

107da65

update ref img

ad713d4

Add documentation

41614a2

Add storybook

8749245

Change step input docs

0ad28d5

Simplify code, change height of cozy mode, update reference image

474dc98

Improve storybook, add is-focus example

f128cda

Add not-focusable buttons inside step input

456ce56

JKMarkowski force-pushed the feat/step-input2 branch from d51059d to 456ce56 Compare May 15, 2020 07:28

rengare approved these changes May 15, 2020

View reviewed changes

salarenko approved these changes May 18, 2020

View reviewed changes

JKMarkowski merged commit 5301d44 into master May 18, 2020

JKMarkowski deleted the feat/step-input2 branch May 18, 2020 09:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Step Input #986

fix: Step Input #986

JKMarkowski commented May 8, 2020

netlify bot commented May 8, 2020 •

edited

Loading

droshev commented May 8, 2020

Vanessa-Cusmich May 11, 2020

Vanessa-Cusmich May 11, 2020

Vanessa-Cusmich May 11, 2020

Vanessa-Cusmich May 11, 2020

JKMarkowski commented May 11, 2020 •

edited

Loading

salarenko May 11, 2020

salarenko May 11, 2020

JKMarkowski May 11, 2020

salarenko May 11, 2020

salarenko May 11, 2020

JKMarkowski May 11, 2020

salarenko May 11, 2020

JKMarkowski commented May 11, 2020

Vanessa-Cusmich left a comment

mikerodonnell89 left a comment

JKMarkowski commented May 14, 2020

prsdthkr commented May 14, 2020

InnaAtanasova left a comment

prsdthkr commented May 14, 2020

JKMarkowski commented May 15, 2020



		## Step Input with Cozy
		Cozy Mode is the default one, it should be used in mobile and tablet devices.



		## Compact Step Input
		Step Input can be used in compact mode, which is done to be used in desktop, or larger screen devices.

fix: Step Input #986

fix: Step Input #986

Conversation

JKMarkowski commented May 8, 2020

Related Issue

Description

Screenshots

Before:

After:

netlify bot commented May 8, 2020 • edited Loading

droshev commented May 8, 2020

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 May 11, 2020 • edited Loading

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 May 11, 2020

Vanessa-Cusmich left a comment

Choose a reason for hiding this comment

mikerodonnell89 left a comment

Choose a reason for hiding this comment

JKMarkowski commented May 14, 2020

prsdthkr commented May 14, 2020

InnaAtanasova left a comment

Choose a reason for hiding this comment

prsdthkr commented May 14, 2020

JKMarkowski commented May 15, 2020

netlify bot commented May 8, 2020 •

edited

Loading

JKMarkowski commented May 11, 2020 •

edited

Loading