Skip to content

Commit

Permalink
Merge branch 'master' into 4243-progress-indicator-dap-violations
Browse files Browse the repository at this point in the history
  • Loading branch information
asudoh authored Oct 9, 2019
2 parents a841b94 + f8da059 commit 1d6d2e0
Show file tree
Hide file tree
Showing 56 changed files with 505 additions and 118 deletions.
118 changes: 118 additions & 0 deletions docs/postmortems/2019-10-07-content-switcher-breaking-change.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
---
date: 2019-10-07
authors: joshblack
---

# Breaking change in 10.6 for `ContentSwitcher`

**Summary**

<!-- What is a one or two-line summary of the event that occurred? -->

A breaking change was introduced in 10.6 for `ContentSwitcher` that impacted
consumers in two ways:

1. The function signature for handlers changed, namely `onChange`
2. The focus implementation lead to focus being stolen whenever a re-render
occured in the control

**Impact**

<!-- What was the scope of impact from the event occuring? How many teams were
impacted? -->

This breaking change impacted all users who downloaded the September release of
the Carbon Design System, notably `10.6.x` and `7.6.x` ranges.

**Root causes**:

<!-- Looking back, what ended up being the main reasons why this event occurred?
-->

We targeted `ContentSwitcher` as a good candidate for a React hooks migration.
Unfortunately, the migration updated parts of the public API in a
semver-incompatible way causing a breaking change.

**Detection**

<!-- How did we find out or discover that this event had occurred? -->

We were notified on GitHub and Slack about this potential breaking change.

**Resolution**

<!-- How did we end up addressing this event in order to mitigate impact? -->

We reverted to the previous behavior of the component using GitHub's UI.

**Action Items**

<!-- What are the action items that came out of this postmortem? Reference
issues and Pull Requests in the "Bug" column with the appropriate owners -->

| Action item | Owner | Bug |
| -------------------------- | ---------- | ---------------------------------------------------------------- |
| Revert to the old behavior | @joshblack | [Link](https://github.com/carbon-design-system/carbon/pull/4250) |

## Lessons learned

**What went well**

- We were able to cleanly revert the breaking change

**What went wrong**

- In the course of refactoring we changed a portion of the public API of a
component that caused breaking changes
- Our response time to a breaking change was long enough that some teams had
implemented a fix already for the broken code
- Even though we noticed a breaking change in the component, we ended up
implementing fixes for in in our website projects instead of addressing the
breaking change

**Where we got lucky**

## Timeline

_Note: times that were hard to determine are marked as 09:00_

2019-09-23 **(all times in UTC-5)**

- 09:00 @justindm234 reported the breaking change as an issue on the Carbon
monorepo

2019-09-30

- 09:00 @emyarod replied with related issues flagged on the Carbon website
properties that were subsequently addressed

2019-10-04

- 06:27 A community on #carbon-react reached out about the possibility of a
breaking change introduced in `carbon-components-react` in `10.6`
- 08:30 @joshblack replied with a link to the existing issue for the breaking
change
- 13:06 @joshblack shared an internal poll to gain feedback on whether or not to
revert the breaking change
- 05:00 @joshblack opened a PR to revert the breaking change identified in the
issue thread. It was not merged as we wanted to verify the fix

2019-10-07

- 08:16 A community member on #carbon-react reached out about a focus-related
bug that was also introducing in the breaking change. In this case, any state
update would trigger the `ContentSwitcher` to steal focus
- 04:00 @joshblack confirmed the bug and decided to open a PR that reverted the
underlying refactor PR for an earlier behavior

## Supporting information

<!-- Any additional information that you might reference earlier on in the
postmortem -->

- [Link](https://github.com/carbon-design-system/carbon/issues/4063) to initial
issue created
- [Link](https://github.com/carbon-design-system/carbon/pull/4230) to Pull
Request for initial bug fix
- [Link](https://github.com/carbon-design-system/carbon/pull/4250) to PR that
reverts the refactoring work done
4 changes: 2 additions & 2 deletions packages/colors/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@carbon/colors",
"description": "Colors for digital and software products using the Carbon Design System",
"version": "10.5.0-rc.0",
"version": "10.5.0",
"license": "Apache-2.0",
"main": "lib/index.js",
"module": "es/index.js",
Expand Down Expand Up @@ -34,7 +34,7 @@
"@carbon/bundler": "10.4.0",
"@carbon/cli-reporter": "10.3.0",
"@carbon/scss-generator": "10.3.0",
"@carbon/test-utils": "10.4.0-rc.0",
"@carbon/test-utils": "10.4.0",
"change-case": "^3.0.2",
"core-js": "^3.0.1",
"fs-extra": "^7.0.0",
Expand Down
3 changes: 3 additions & 0 deletions packages/components/docs/sass.md
Original file line number Diff line number Diff line change
Expand Up @@ -10361,6 +10361,7 @@ Get the font-family for an IBM Plex font
- [carbon--font-family [mixin]](#carbon--font-family-mixin)
- [form [mixin]](#form-mixin)
- [number-input [mixin]](#number-input-mixin)
- [slider [mixin]](#slider-mixin)

### ✅carbon--font-family [mixin]

Expand Down Expand Up @@ -21393,6 +21394,7 @@ Slider styles

.#{$prefix}--slider__range-label {
@include type-style('code-02');
font-family: carbon--font-family('mono');
color: $text-01;

&:last-of-type {
Expand Down Expand Up @@ -21569,6 +21571,7 @@ Slider styles

- **Group**: [slider](#slider)
- **Requires**:
- [carbon--font-family [function]](#carbon--font-family-function)
- [prefix [variable]](#prefix-variable)
- [carbon--spacing-05 [variable]](#carbon--spacing-05-variable)
- [text-01 [variable]](#text-01-variable)
Expand Down
6 changes: 3 additions & 3 deletions packages/components/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "carbon-components",
"description": "The Carbon Design System is IBM’s open-source design system for products and experiences.",
"version": "10.7.0-rc.0",
"version": "10.7.0",
"license": "Apache-2.0",
"main": "umd/index.js",
"module": "es/index.js",
Expand Down Expand Up @@ -80,10 +80,10 @@
"@babel/preset-env": "^7.0.0",
"@babel/preset-react": "^7.0.0",
"@babel/runtime": "^7.5.2",
"@carbon/elements": "10.7.0-rc.0",
"@carbon/elements": "10.7.0",
"@carbon/icons-handlebars": "10.6.0",
"@carbon/icons-react": "10.6.0",
"@carbon/test-utils": "10.4.0-rc.0",
"@carbon/test-utils": "10.4.0",
"@frctl/fractal": "^1.1.0",
"adaro": "1.0.4",
"autoprefixer": "^8.2.0",
Expand Down
6 changes: 4 additions & 2 deletions packages/components/src/components/button/_button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,12 @@
}

&:focus {
color: $text-04;
color: $inverse-01;
background-color: $interactive-03;
}

&:active {
background-color: $active-primary;
color: $inverse-01;
}

&:disabled,
Expand All @@ -112,6 +112,7 @@
&.#{$prefix}--btn--disabled:focus {
background: transparent;
color: $disabled;
outline: none;

& > .#{$prefix}--btn__icon path {
fill: $disabled;
Expand Down Expand Up @@ -161,6 +162,7 @@
color: $disabled;
background: transparent;
border-color: transparent;
outline: none;

.#{$prefix}--btn__icon path {
fill: $disabled;
Expand Down
20 changes: 13 additions & 7 deletions packages/components/src/components/checkbox/_checkbox.scss
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,18 @@
@mixin checkbox {
// Spacing between checkboxes
.#{$prefix}--form-item.#{$prefix}--checkbox-wrapper {
margin-bottom: rem(8px);
margin-bottom: $carbon--spacing-02;
}

// Spacing above collection of checkboxes
.#{$prefix}--form-item.#{$prefix}--checkbox-wrapper:first-of-type {
margin-top: rem(3px);
}

// Remove spacing above collection of checkboxes if label is present
// Shift collection of checkboxes up if label is present
// to account for the 2px top margin for the first checkbox
.#{$prefix}--label + .#{$prefix}--form-item.#{$prefix}--checkbox-wrapper {
margin-top: 0;
margin-top: -#{$carbon--spacing-01};
}

// Spacing below collection of checkboxes
Expand All @@ -48,7 +49,6 @@
.#{$prefix}--checkbox-label {
@include reset;
@include type-style('body-short-01');

line-height: 1.5rem;
position: relative;
display: flex;
Expand All @@ -58,9 +58,14 @@
user-select: none;
}

// Required because `$css--reset: true` cannot currently apply to this `::before` and `::after`
.#{$prefix}--checkbox-label::before,
.#{$prefix}--checkbox-label::after {
box-sizing: border-box;
}

// Spacing for presentational checkbox
.#{$prefix}--checkbox-label::before {
box-sizing: border-box;
content: '';

// According to the spec, we'll want the bounding box for our checkbox to
Expand All @@ -87,11 +92,12 @@
// Create the appearance of the check in the `after` pseudo-element
.#{$prefix}--checkbox-label::after {
content: '';

position: absolute;
left: rem(6px);
top: rem(8px);
width: rem(7px);
height: rem(3px);
width: rem(9px);
height: rem(5px);
background: none;
border-left: 2px solid $inverse-01;
border-bottom: 2px solid $inverse-01;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@
//TOOLBAR
//-------------------------------------------------
.#{$prefix}--table-toolbar {
display: flex;
width: 100%;
background: $ui-01;
display: flex;
height: $layout-04;
overflow: hidden;
position: relative; //need for batch actions
width: 100%;
}

.#{$prefix}--toolbar-content {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
.#{$prefix}--date-picker-input__wrapper {
display: flex;
align-items: center;
position: relative;

~ .#{$prefix}--form-requirement {
max-height: rem(200px);
Expand Down Expand Up @@ -130,6 +131,9 @@
fill: $icon-01;
cursor: pointer;
z-index: 1;
// vertically center icon within parent container on IE11
top: 50%;
transform: translateY(-50%);
}

.#{$prefix}--date-picker__icon ~ .#{$prefix}--date-picker__input {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@
margin-top: rem(6px);
}

// Remove spacing above collection of radio buttons if label is present
.#{$prefix}--label + .#{$prefix}--form-item .#{$prefix}--radio-button-group {
margin-top: 0;
}

// vertical radio button
.#{$prefix}--radio-button-group--vertical {
flex-direction: column;
Expand All @@ -37,6 +42,7 @@

.#{$prefix}--radio-button__label {
margin-right: 0;
line-height: carbon--mini-units(2.5);
}

.#{$prefix}--radio-button__label:not(:last-of-type) {
Expand Down
3 changes: 2 additions & 1 deletion packages/components/src/components/search/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ Use these modifiers with `.bx--search` class.
| Name | Description |
| -------------------- | -------------------------------------------- |
| `.bx--search--sm` | Selector for applying small search styles |
| `.bx--search--lg` | Selector for applying standard search styles |
| `.bx--search--lg` | Selector for applying medium search styles |
| `.bx--search--xl` | Selector for applying standard search styles |
| `.bx--search--light` | Selector for applying light search styles |

### JavaScript
Expand Down
14 changes: 14 additions & 0 deletions packages/components/src/components/search/_search.scss
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@
height: rem(32px);
}

.#{$prefix}--search--lg .#{$prefix}--search-input {
@include type-style('body-short-02');
height: rem(40px);
}

.#{$prefix}--search--xl .#{$prefix}--search-input {
@include type-style('body-short-02');
height: rem(48px);
Expand Down Expand Up @@ -203,6 +208,14 @@
}
}

.#{$prefix}--search--lg {
.#{$prefix}--search-close,
~ .#{$prefix}--search-button {
height: rem(40px);
width: rem(40px);
}
}

.#{$prefix}--search--xl {
.#{$prefix}--search-close,
~ .#{$prefix}--search-button {
Expand All @@ -216,6 +229,7 @@
opacity: 0;
}

.#{$prefix}--search--xl.#{$prefix}--skeleton .#{$prefix}--search-input,
.#{$prefix}--search--lg.#{$prefix}--skeleton .#{$prefix}--search-input,
.#{$prefix}--search--sm.#{$prefix}--skeleton .#{$prefix}--search-input {
@include skeleton;
Expand Down
Loading

0 comments on commit 1d6d2e0

Please sign in to comment.