From afd30d7e36a0c070e167db861f977d67afd2865e Mon Sep 17 00:00:00 2001 From: emyarod Date: Mon, 1 Mar 2021 10:48:51 -0600 Subject: [PATCH 1/4] fix(NumberInput): deprecate mobile variant (#7919) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- .../number-input/_number-input.scss | 60 ---------------- .../NumberInput/NumberInput-story.js | 1 - .../src/components/NumberInput/NumberInput.js | 69 ++++++++----------- 3 files changed, 29 insertions(+), 101 deletions(-) diff --git a/packages/components/src/components/number-input/_number-input.scss b/packages/components/src/components/number-input/_number-input.scss index 67ee806c78a2..0788c4d0fecc 100644 --- a/packages/components/src/components/number-input/_number-input.scss +++ b/packages/components/src/components/number-input/_number-input.scss @@ -381,54 +381,6 @@ background-color: $hover-light-ui; } - .#{$prefix}--number--mobile { - width: auto; - min-width: rem(144px); - - .#{$prefix}--number__control-btn, - &.#{$prefix}--number--light .#{$prefix}--number__control-btn { - position: static; - width: rem(40px); - height: rem(40px); - background-color: $ui-01; - - &:hover, - &:focus { - background-color: $hover-ui; - } - - &:focus { - outline-width: 2px; - outline-offset: -2px; - } - - svg { - position: static; - } - } - - input[type='number'] { - width: auto; - min-width: rem(64px); - margin: 0; - padding: 0; - text-align: center; - background-color: $field-01; - border-right: 1px solid $ui-03; - border-left: 1px solid $ui-03; - } - - &.#{$prefix}--number--light { - input[type='number'] { - background-color: $field-02; - } - - .#{$prefix}--number__control-btn { - background-color: $ui-02; - } - } - } - // Size Variant styles .#{$prefix}--number--xl input[type='number'] { height: rem(48px); @@ -448,12 +400,6 @@ } } - .#{$prefix}--number--xl.#{$prefix}--number--mobile - .#{$prefix}--number__control-btn { - width: rem(48px); - height: rem(48px); - } - .#{$prefix}--number--sm input[type='number'] { height: rem(32px); } @@ -472,12 +418,6 @@ } } - .#{$prefix}--number--sm.#{$prefix}--number--mobile - .#{$prefix}--number__control-btn { - width: rem(32px); - height: rem(32px); - } - //No label positioning adjustment .#{$prefix}--number--nolabel .bx--label + .bx--form__helper-text { margin-top: 0; diff --git a/packages/react/src/components/NumberInput/NumberInput-story.js b/packages/react/src/components/NumberInput/NumberInput-story.js index fd7dd1b4ac49..73d167ac0c3e 100644 --- a/packages/react/src/components/NumberInput/NumberInput-story.js +++ b/packages/react/src/components/NumberInput/NumberInput-story.js @@ -40,7 +40,6 @@ const props = () => ({ disabled: boolean('Disabled (disabled)', false), readOnly: boolean('Read only (readOnly)', false), invalid: boolean('Show form validation UI (invalid)', false), - isMobile: boolean('Mobile variant', false), invalidText: text( 'Form validation UI content (invalidText)', 'Number is not valid' diff --git a/packages/react/src/components/NumberInput/NumberInput.js b/packages/react/src/components/NumberInput/NumberInput.js index 5ef4db599295..2b0749fe8fae 100644 --- a/packages/react/src/components/NumberInput/NumberInput.js +++ b/packages/react/src/components/NumberInput/NumberInput.js @@ -18,6 +18,7 @@ import { import mergeRefs from '../../tools/mergeRefs'; import requiredIfValueExists from '../../prop-types/requiredIfValueExists'; import { useControlledStateWithValue } from '../../internal/FeatureFlags'; +import deprecate from '../../prop-types/deprecate'; const { prefix } = settings; @@ -46,67 +47,86 @@ class NumberInput extends Component { * `true` to allow empty string. */ allowEmpty: PropTypes.bool, + /** * Provide a description that would be used to best describe the use case of the NumberInput component */ ariaLabel: PropTypes.string, + /** * Specify an optional className to be applied to the wrapper node */ className: PropTypes.string, + /** * Optional starting value for uncontrolled state */ defaultValue: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), + /** * Specify if the control should be disabled, or not */ disabled: PropTypes.bool, + /** * Provide text that is used alongside the control label for additional help */ helperText: PropTypes.node, + /** * Specify whether you want the underlying label to be visually hidden */ hideLabel: PropTypes.bool, + /** * Provide a description for up/down icons that can be read by screen readers */ iconDescription: PropTypes.string.isRequired, + /** * Specify a custom `id` for the input */ id: PropTypes.string.isRequired, + /** * Specify if the currently value is invalid. */ invalid: PropTypes.bool, + /** * Message which is displayed if the value is invalid. */ invalidText: PropTypes.node, + /** * `true` to use the mobile variant. */ - isMobile: PropTypes.bool, + isMobile: deprecate( + PropTypes.bool, + `The \`isMobile\` prop no longer needed as the default NumberInput styles are now identical to the mobile variant styles. This prop will be removed in the next major version of \`carbon-components-react\`` + ), + /** * Generic `label` that will be used as the textual representation of what * this field is for */ label: PropTypes.node, + /** * `true` to use the light version. */ light: PropTypes.bool, + /** * The maximum value. */ max: PropTypes.number, + /** * The minimum value. */ min: PropTypes.number, + /** * The new value is available in 'imaginaryTarget.value' * i.e. to get the value: evt.imaginaryTarget.value @@ -120,34 +140,42 @@ class NumberInput extends Component { onChange: !useControlledStateWithValue ? PropTypes.func : requiredIfValueExists(PropTypes.func), + /** * Provide an optional function to be called when the up/down button is clicked */ onClick: PropTypes.func, + /** * Specify if the component should be read-only */ readOnly: PropTypes.bool, + /** * Specify the size of the Number Input. Currently supports either `sm` or `xl` as an option. */ size: PropTypes.oneOf(['sm', 'xl']), + /** * Specify how much the valus should increase/decrease upon clicking on up/down button */ step: PropTypes.number, + /** * Provide custom text for the component for each translation id */ translateWithId: PropTypes.func.isRequired, + /** * Specify the value of the input */ value: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), + /** * Specify whether the control is currently in warning state */ warn: PropTypes.bool, + /** * Provide the text that is displayed when the control is in warning state */ @@ -411,45 +439,6 @@ class NumberInput extends Component {
{(() => { - if (isMobile) { - return ( - <> - {labelText} - {helper} -
- - - -
- - ); - } return ( <> {labelText} From 3861277417fb46bf025f0ad536486ec68bce9255 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 1 Mar 2021 14:50:15 -0600 Subject: [PATCH 2/4] feat(feature-flags): rename exports, add merge, and update tests (#7802) * feat(feature-flags): rename exports, add merge, and update tests * feat(feature-flags): add support for process.env checking for flags * feat(feature-flags): add sass module variant with behavior that matches JS * feat(project): integrate feature-flags with react package * docs(feature-flags): update README Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- packages/feature-flags/README.md | 103 ++++++------ .../__tests__/feature-flags-test.js | 40 ----- packages/feature-flags/__tests__/scss-test.js | 157 ++++++++++++++---- packages/feature-flags/feature-flags.yml | 5 + packages/feature-flags/index.scss | 64 +++++++ packages/feature-flags/package.json | 7 +- .../feature-flags/scss/feature-flags.scss | 30 ---- .../src/__tests__/feature-flags-test.js | 104 ++++++++++++ packages/feature-flags/src/index.js | 22 ++- packages/feature-flags/tasks/build.js | 13 +- packages/react/package.json | 1 + packages/react/src/internal/FeatureFlags.js | 6 +- yarn.lock | 4 +- 13 files changed, 389 insertions(+), 167 deletions(-) delete mode 100644 packages/feature-flags/__tests__/feature-flags-test.js create mode 100644 packages/feature-flags/index.scss delete mode 100644 packages/feature-flags/scss/feature-flags.scss create mode 100644 packages/feature-flags/src/__tests__/feature-flags-test.js diff --git a/packages/feature-flags/README.md b/packages/feature-flags/README.md index 939018cb1585..966d2fd7a335 100644 --- a/packages/feature-flags/README.md +++ b/packages/feature-flags/README.md @@ -24,83 +24,76 @@ The `@carbon/feature-flags` provides a runtime-based feature flag system that you can use to enable or disable experimental features from Carbon or in your own code. -To enable a feature flag, you will need to enable it in each of the environments -that you're using it in. Often times, this will mean JavaScript, Sass, or both. - -To enable a flag in JavaScript, you would use `enableFeatureFlag`: +To check if a feature flag is enabled, you can use the `enabled` function in +JavaScript: ```js -import { enableFeatureFlag } from '@carbon/feature-flags'; +import { enabled } from '@carbon/feature-flags'; -enableFeatureFlag('feature-flag-name'); +enabled('feature-flag-name'); ``` -To enable a flag in Sass, you would set the `$feature-flags` variable: +In Sass, you would use the `enabled` function or mixin: ```scss -@import '@carbon/feature-flags/scss/feature-flags'; +@use '@carbon/feature-flags'; -$feature-flags: map-merge( - $feature-flags, - ( - 'feature-flag-name': true, - ) -); -``` +// Return true if the flag is enabled +@if feature-flags.enabled('feature-flag-name') { + // +} -### Managing and using feature flags +@include enabled('feature-flag-name') { + // only include contents if the flag is enabled +} +``` -You can use the `@carbon/feature-flags` package to build on top of existing -feature flags, or to add your own. +### Managing feature flags -You can add and toggle flags in JavaScript and Sass. In JavaScript, this would -look like: +You can change whether a feature flag is enabled. In JavaScript, you can use the +`enable`, `disable`, and `merge` functions to accomplish this. ```js -import { - addFeatureFlag, - enableFeatureFlag, - disableFeatureFlag, - featureFlagEnabled, -} from '@carbon/feature-flags'; - -// Specify a default value for the flag -addFeatureFlag('feature-flag-name', false); - -// You can use `featureFlagEnabled` to conditionally run -// branches of your code -if (featureFlagEnabled('feature-flag-name')) { - // Run code if the flag is enabled -} +import { enable, disable, merge } from '@carbon/feature-flags'; -// You can also modify the value of the flag -disableFeatureFlag('feature-flag-name'); -enableFeatureFlag('feature-flag-name'); +// Enable `feature-flag-a` +enable('feature-flag-a'); + +// Disable `feature-flag-a` +disable('feature-flag-a); + +// Set a variety of feature flags to a specific value +merge({ + 'feature-flag-a': true, + 'feature-flag-b': false, + 'feature-flag-c': true, +}); ``` -In Sass, you would write the following: +In Sass, you can configure whether a feature flag is enabled when you include +the module or by using `enable`, `disable`, and `merge`. ```scss -@import '@carbon/feature-flags/scss/feature-flags'; +@use '@carbon/feature-flags' with ( + $feature-flags: ( + 'feature-flag-a': false, + 'feature-flag-b': true, + ), +); + +// Enable `feature-flag-a` +@include feature-flags.enable('feature-flag-a'); + +// Disable `feature-flag-b` +@include feature-flags.disable('feature-flag-b'); -$feature-flags: map-merge( - $feature-flags, +// Set a variety of feature flags to a specific value +@include feature-flags.merge( ( - 'feature-flag-name': true, + 'feature-flag-a': true, + 'feature-flag-b': true, ) ); - -@if feature-flag-enabled('feature-flag-name') { - // ... -} - -// You can also run this as a mixin to conditionally include -// code -.my-selector { - @include feature-flag-enabled('feature-flag-name') { - // ... - } -} ``` ## 🙌 Contributing diff --git a/packages/feature-flags/__tests__/feature-flags-test.js b/packages/feature-flags/__tests__/feature-flags-test.js deleted file mode 100644 index 236ef6ecc7a9..000000000000 --- a/packages/feature-flags/__tests__/feature-flags-test.js +++ /dev/null @@ -1,40 +0,0 @@ -/** - * Copyright IBM Corp. 2015, 2020 - * - * This source code is licensed under the Apache-2.0 license found in the - * LICENSE file in the root directory of this source tree. - */ - -import { - addFeatureFlag, - enableFeatureFlag, - disableFeatureFlag, - featureFlagEnabled, -} from '../src'; - -describe('@carbon/feature-flags', () => { - it('should let the user check if a feature flag is enabled', () => { - addFeatureFlag('test', false); - expect(featureFlagEnabled('test')).toBe(false); - - enableFeatureFlag('test'); - expect(featureFlagEnabled('test')).toBe(true); - - disableFeatureFlag('test'); - expect(featureFlagEnabled('test')).toBe(false); - }); - - it('should throw an error if a flag does not exist', () => { - expect(() => { - enableFeatureFlag('does-not-exist'); - }).toThrow(); - - expect(() => { - disableFeatureFlag('does-not-exist'); - }).toThrow(); - - expect(() => { - featureFlagEnabled('does-not-exist'); - }).toThrow(); - }); -}); diff --git a/packages/feature-flags/__tests__/scss-test.js b/packages/feature-flags/__tests__/scss-test.js index 8fc3d72d6dda..129e472f02b4 100644 --- a/packages/feature-flags/__tests__/scss-test.js +++ b/packages/feature-flags/__tests__/scss-test.js @@ -9,49 +9,142 @@ 'use strict'; -const { convert, createSassRenderer } = require('@carbon/test-utils/scss'); +const { SassRenderer } = require('@carbon/test-utils/scss'); -const render = createSassRenderer(__dirname); +const { render } = SassRenderer.create(__dirname); -describe('feature-flags.scss', () => { - it('should support default feature flags before the import', async () => { - const { calls } = await render(` - $feature-flags: ('test': true); +describe('@carbon/feature-flags', () => { + describe('add', () => { + it('should add the given flag and set whether its enabled', async () => { + const { getValue } = await render(` + @use '../index.scss' as feature-flags; - @import '../scss/feature-flags'; + @include feature-flags.add(flag-a, true); + @include feature-flags.add(flag-b, false); - @if feature-flag-enabled('test') { - $t: test(true); - } - `); + $_: get-value(feature-flags.enabled(flag-a)); + $_: get-value(feature-flags.enabled(flag-b)); + `); - expect(calls.length).toBe(1); - expect(convert(calls[0][0])).toBe(true); + // flag-a + expect(getValue(0)).toBe(true); + // flag-b + expect(getValue(1)).toBe(false); + }); }); - it('should support modifying flags', async () => { - const { calls } = await render(` - @import '../scss/feature-flags'; + describe('enable', () => { + it('should enable the given feature flag', async () => { + const { getValue } = await render(` + @use '../index.scss' as feature-flags; - $feature-flags: map-merge($feature-flags, ( - 'test': true, - )); + @include feature-flags.add(flag-a, false); - @if feature-flag-enabled('test') { - $t: test('feature-flag-enabled'); - } + $_: get-value(feature-flags.enabled(flag-a)); - $feature-flags: map-merge($feature-flags, ( - 'test': false, - )); + @include feature-flags.enable(flag-a); - @if not feature-flag-enabled('test') { - $t: test('feature-flag-disabled'); - } - `); + $_: get-value(feature-flags.enabled(flag-a)); + `); - expect(calls.length).toBe(2); - expect(convert(calls[0][0])).toBe('feature-flag-enabled'); - expect(convert(calls[1][0])).toBe('feature-flag-disabled'); + expect(getValue(0)).toBe(false); + expect(getValue(1)).toBe(true); + }); }); + + describe('disable', () => { + it('should disable the given feature flag', async () => { + const { getValue } = await render(` + @use '../index.scss' as feature-flags; + + @include feature-flags.add(flag-a, true); + + $_: get-value(feature-flags.enabled(flag-a)); + + @include feature-flags.disable(flag-a); + + $_: get-value(feature-flags.enabled(flag-a)); + `); + + expect(getValue(0)).toBe(true); + expect(getValue(1)).toBe(false); + }); + }); + + describe('enabled', () => { + it('should return whether a flag is enabled or disabled', async () => { + const { getValue } = await render(` + @use '../index.scss' as feature-flags; + + @include feature-flags.add(flag-a, true); + @include feature-flags.add(flag-b, false); + + $_: get-value(feature-flags.enabled(flag-a)); + $_: get-value(feature-flags.enabled(flag-b)); + `); + + // flag-a + expect(getValue(0)).toBe(true); + // flag-b + expect(getValue(1)).toBe(false); + }); + }); + + describe('merge', () => { + it('should set each feature flag given', async () => { + const { getValue } = await render(` + @use '../index.scss' as feature-flags; + + @include feature-flags.add(flag-c, false); + @include feature-flags.merge(( + flag-a: true, + flag-b: false, + flag-c: true, + )); + + $_: get-value(feature-flags.enabled(flag-a)); + $_: get-value(feature-flags.enabled(flag-b)); + $_: get-value(feature-flags.enabled(flag-c)); + `); + + // flag-a + expect(getValue(0)).toBe(true); + // flag-b + expect(getValue(1)).toBe(false); + // flag-c + expect(getValue(2)).toBe(true); + }); + }); + + // it('should support default feature flags before the import', async () => { + // const { calls } = await render(` + // $feature-flags: ('test': true); + // @import '../scss/feature-flags'; + // @if feature-flag-enabled('test') { + // $t: test(true); + // } + // `); + // expect(calls.length).toBe(1); + // expect(convert(calls[0][0])).toBe(true); + // }); + // it('should support modifying flags', async () => { + // const { calls } = await render(` + // @import '../scss/feature-flags'; + // $feature-flags: map-merge($feature-flags, ( + // 'test': true, + // )); + // @if feature-flag-enabled('test') { + // $t: test('feature-flag-enabled'); + // } + // $feature-flags: map-merge($feature-flags, ( + // 'test': false, + // )); + // @if not feature-flag-enabled('test') { + // $t: test('feature-flag-disabled'); + // } + // `); + // expect(calls.length).toBe(2); + // expect(convert(calls[0][0])).toBe('feature-flag-enabled'); + // expect(convert(calls[1][0])).toBe('feature-flag-disabled'); + // }); }); diff --git a/packages/feature-flags/feature-flags.yml b/packages/feature-flags/feature-flags.yml index 59a369e4acdc..333f2d4f3ab6 100644 --- a/packages/feature-flags/feature-flags.yml +++ b/packages/feature-flags/feature-flags.yml @@ -9,3 +9,8 @@ feature-flags: - name: enable-css-custom-properties description: Describe what the flag does enabled: false + - name: enable-use-controlled-state-with-value + description: > + Enable components to be created in either a controlled or uncontrolled + mode + enabled: false diff --git a/packages/feature-flags/index.scss b/packages/feature-flags/index.scss new file mode 100644 index 000000000000..2725779c0f28 --- /dev/null +++ b/packages/feature-flags/index.scss @@ -0,0 +1,64 @@ +// +// Copyright IBM Corp. 2015, 2020 +// +// This source code is licensed under the Apache-2.0 license found in the +// LICENSE file in the root directory of this source tree. +// + +@use 'sass:map'; +@use 'scss/generated/feature-flags'; + +$feature-flags: () !default; +$feature-flags: map.merge( + feature-flags.$generated-feature-flags, + $feature-flags +); + +@function check-for-flag($name) { + @if map.has-key($feature-flags, $name) == true { + @return true; + } + @error 'Unable to find a feature flag named: #{$name}'; +} + +@mixin add($name, $enabled: false) { + @if map.has-key($feature-flags, $name) == true { + @error 'A feature flag named #{$name} already exists'; + } + + $feature-flags: map.set($feature-flags, $name, $enabled) !global; +} + +@mixin enable($name) { + @if check-for-flag($name) == true { + $feature-flags: map.set($feature-flags, $name, true) !global; + } +} + +@mixin disable($name) { + @if check-for-flag($name) == true { + $feature-flags: map.set($feature-flags, $name, false) !global; + } +} + +@mixin merge($flags) { + $feature-flags: map.merge($feature-flags, $flags) !global; +} + +/// Check if a feature flag is enabled +/// @param {String} $name +/// @returns {Boolean} +@function enabled($name) { + @if check-for-flag($name) == true { + @return map.get($feature-flags, $name); + } +} + +/// Emit the content of the mixin if the feature flag is enabled +/// @param {String} $name +/// @content +@mixin enabled($name) { + @if enabled($name) { + @content; + } +} diff --git a/packages/feature-flags/package.json b/packages/feature-flags/package.json index 4d411f0f96cc..848a287905ee 100644 --- a/packages/feature-flags/package.json +++ b/packages/feature-flags/package.json @@ -1,8 +1,7 @@ { "name": "@carbon/feature-flags", - "private": true, "description": "Build with feature flags in Carbon", - "version": "0.5.0", + "version": "0.0.0", "license": "Apache-2.0", "main": "lib/index.js", "module": "es/index.js", @@ -28,10 +27,12 @@ "@babel/generator": "^7.10.2", "@babel/types": "^7.10.2", "@carbon/scss-generator": "^10.13.0", + "change-case": "^4.1.2", "fs-extra": "^9.0.1", "js-yaml": "^3.14.0", "rimraf": "^3.0.2", "rollup": "^2.38.0", "rollup-plugin-strip-banner": "^2.0.0" - } + }, + "sideEffects": false } diff --git a/packages/feature-flags/scss/feature-flags.scss b/packages/feature-flags/scss/feature-flags.scss deleted file mode 100644 index f73c95d9b50f..000000000000 --- a/packages/feature-flags/scss/feature-flags.scss +++ /dev/null @@ -1,30 +0,0 @@ -// -// Copyright IBM Corp. 2015, 2020 -// -// This source code is licensed under the Apache-2.0 license found in the -// LICENSE file in the root directory of this source tree. -// - -@import 'generated/feature-flags'; - -$feature-flags: () !default; -$feature-flags: map-merge($generated-feature-flags, $feature-flags); - -/// Check if a feature flag is enabled -/// @param {String} $name -/// @returns {Boolean} -@function feature-flag-enabled($name) { - @if (map-has-key($feature-flags, $name)) { - @return map-get($feature-flags, $name); - } - @error 'Unable to find feature flag named #{$name}'; -} - -/// Emit the content of the mixin if the feature flag is enabled -/// @param {String} $name -/// @content -@mixin feature-flag-enabled($name) { - @if feature-flag-enabled($name) { - @content; - } -} diff --git a/packages/feature-flags/src/__tests__/feature-flags-test.js b/packages/feature-flags/src/__tests__/feature-flags-test.js new file mode 100644 index 000000000000..45e3dfb89c74 --- /dev/null +++ b/packages/feature-flags/src/__tests__/feature-flags-test.js @@ -0,0 +1,104 @@ +/** + * Copyright IBM Corp. 2015, 2020 + * + * This source code is licensed under the Apache-2.0 license found in the + * LICENSE file in the root directory of this source tree. + */ + +describe('@carbon/feature-flags', () => { + let FeatureFlag; + + beforeEach(() => { + jest.resetModules(); + FeatureFlag = require('../'); + }); + + describe('add', () => { + it('should add the given flag and set whether its enabled', () => { + FeatureFlag.add('flag-a', true); + FeatureFlag.add('flag-b', false); + + expect(FeatureFlag.enabled('flag-a')).toBe(true); + expect(FeatureFlag.enabled('flag-b')).toBe(false); + }); + + it('should throw if a duplicate flag name is given', () => { + FeatureFlag.add('flag-a', true); + + expect(() => { + FeatureFlag.add('flag-a', true); + }).toThrow(); + }); + }); + + describe('enable', () => { + it('should enable the given feature flag', () => { + FeatureFlag.add('flag-a', false); + expect(FeatureFlag.enabled('flag-a')).toBe(false); + + FeatureFlag.enable('flag-a'); + expect(FeatureFlag.enabled('flag-a')).toBe(true); + }); + + it('should throw if the given flag does not exist', () => { + expect(() => { + FeatureFlag.enable('flag-a'); + }).toThrow(); + }); + }); + + describe('disable', () => { + it('should disable the given feature flag', () => { + FeatureFlag.add('flag-a', true); + expect(FeatureFlag.enabled('flag-a')).toBe(true); + + FeatureFlag.disable('flag-a'); + expect(FeatureFlag.enabled('flag-a')).toBe(false); + }); + + it('should throw if the given flag does not exist', () => { + expect(() => { + FeatureFlag.disable('flag-a'); + }).toThrow(); + }); + }); + + describe('enabled', () => { + it('should return whether a flag is enabled or disabled', () => { + FeatureFlag.add('flag-a', true); + FeatureFlag.add('flag-b', false); + + expect(FeatureFlag.enabled('flag-a')).toBe(true); + expect(FeatureFlag.enabled('flag-b')).toBe(false); + }); + + it('should throw if the given flag does not exist', () => { + expect(() => { + FeatureFlag.enabled('flag-a'); + }).toThrow(); + }); + }); + + describe('merge', () => { + it('should set each feature flag given', () => { + FeatureFlag.merge({ + 'flag-a': true, + 'flag-b': false, + }); + + expect(FeatureFlag.enabled('flag-a')).toBe(true); + expect(FeatureFlag.enabled('flag-b')).toBe(false); + }); + + it('should override duplicate keys with the given flag', () => { + FeatureFlag.add('flag-b', true); + FeatureFlag.merge({ + 'flag-a': true, + 'flag-b': false, + }); + + expect(FeatureFlag.enabled('flag-a')).toBe(true); + expect(FeatureFlag.enabled('flag-b')).toBe(false); + }); + }); +}); diff --git a/packages/feature-flags/src/index.js b/packages/feature-flags/src/index.js index b4c2da9f630d..b6119e1c2b30 100644 --- a/packages/feature-flags/src/index.js +++ b/packages/feature-flags/src/index.js @@ -29,7 +29,10 @@ function checkForFlag(name) { * @param {string} name * @param {boolean} enabled */ -export function addFeatureFlag(name, enabled) { +export function add(name, enabled) { + if (featureFlags.has(name)) { + throw new Error(`The feature flag: ${name} already exists`); + } featureFlags.set(name, enabled); } @@ -37,7 +40,7 @@ export function addFeatureFlag(name, enabled) { * Enable a feature flag * @param {string} name */ -export function enableFeatureFlag(name) { +export function enable(name) { checkForFlag(name); featureFlags.set(name, true); } @@ -46,17 +49,28 @@ export function enableFeatureFlag(name) { * Disable a feature flag * @param {string} name */ -export function disableFeatureFlag(name) { +export function disable(name) { checkForFlag(name); featureFlags.set(name, false); } +/** + * Merge the given feature flags with the current set of feature flags. + * Duplicate keys will be set to the value in the given feature flags. + * @param {object} flags + */ +export function merge(flags) { + Object.keys(flags).forEach((key) => { + featureFlags.set(key, flags[key]); + }); +} + /** * Check if a feature flag is enabled * @param {string} name * @returns {boolean} */ -export function featureFlagEnabled(name) { +export function enabled(name) { checkForFlag(name); return featureFlags.get(name); } diff --git a/packages/feature-flags/tasks/build.js b/packages/feature-flags/tasks/build.js index 01919115b2b6..8589bc8a910b 100644 --- a/packages/feature-flags/tasks/build.js +++ b/packages/feature-flags/tasks/build.js @@ -10,6 +10,7 @@ const { default: babelGenerate } = require('@babel/generator'); const babelTypes = require('@babel/types'); const { types: t, generate } = require('@carbon/scss-generator'); +const { constantCase } = require('change-case'); const fs = require('fs-extra'); const yaml = require('js-yaml'); const path = require('path'); @@ -96,7 +97,17 @@ function buildJavaScriptModule(featureFlags) { ), t.objectProperty( t.identifier('enabled'), - t.booleanLiteral(featureFlag.enabled) + t.logicalExpression( + '||', + t.memberExpression( + t.memberExpression( + t.identifier('process'), + t.identifier('env') + ), + t.identifier(constantCase(`CARBON_${featureFlag.name}`)) + ), + t.booleanLiteral(featureFlag.enabled) + ) ), ]); }) diff --git a/packages/react/package.json b/packages/react/package.json index 3d22de3382dc..36fe5ae33f93 100644 --- a/packages/react/package.json +++ b/packages/react/package.json @@ -44,6 +44,7 @@ "react-dom": "^16.8.6 || ^17.0.1" }, "dependencies": { + "@carbon/feature-flags": "^0.0.0", "@carbon/icons-react": "^10.26.0", "@carbon/telemetry": "0.0.0-alpha.6", "classnames": "2.2.6", diff --git a/packages/react/src/internal/FeatureFlags.js b/packages/react/src/internal/FeatureFlags.js index de593328aee8..48fe0108ea57 100644 --- a/packages/react/src/internal/FeatureFlags.js +++ b/packages/react/src/internal/FeatureFlags.js @@ -5,6 +5,8 @@ * LICENSE file in the root directory of this source tree. */ +import { enabled } from '@carbon/feature-flags'; + /** * This file contains the list of the default values of compile-time feature flags. * @@ -44,4 +46,6 @@ * * `rest` tells you additional information based on the source component * * _Without_ this feature flag the event handler has component-specific signature, e.g. `onChange(event, direction)`. */ -export const useControlledStateWithValue = false; +export const useControlledStateWithValue = enabled( + 'enable-use-controlled-state-with-value' +); diff --git a/yarn.lock b/yarn.lock index 2762b7b9e7ff..7bcc67636fef 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1676,13 +1676,14 @@ __metadata: languageName: unknown linkType: soft -"@carbon/feature-flags@workspace:packages/feature-flags": +"@carbon/feature-flags@^0.0.0, @carbon/feature-flags@workspace:packages/feature-flags": version: 0.0.0-use.local resolution: "@carbon/feature-flags@workspace:packages/feature-flags" dependencies: "@babel/generator": ^7.10.2 "@babel/types": ^7.10.2 "@carbon/scss-generator": ^10.13.0 + change-case: ^4.1.2 fs-extra: ^9.0.1 js-yaml: ^3.14.0 rimraf: ^3.0.2 @@ -9229,6 +9230,7 @@ __metadata: "@babel/plugin-transform-object-assign": ^7.7.4 "@babel/preset-env": ^7.10.0 "@babel/preset-react": ^7.10.0 + "@carbon/feature-flags": ^0.0.0 "@carbon/icons-react": ^10.26.0 "@carbon/telemetry": 0.0.0-alpha.6 "@carbon/test-utils": ^10.15.0 From 0cb4b3022b0f599f5be62b27015fee60b698d65b Mon Sep 17 00:00:00 2001 From: carbon-bot Date: Mon, 1 Mar 2021 21:15:57 +0000 Subject: [PATCH 3/4] chore(project): sync generated files --- packages/feature-flags/package.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/feature-flags/package.json b/packages/feature-flags/package.json index 848a287905ee..9c600cd71c49 100644 --- a/packages/feature-flags/package.json +++ b/packages/feature-flags/package.json @@ -18,6 +18,9 @@ "components", "react" ], + "publishConfig": { + "access": "public" + }, "scripts": { "build": "yarn clean && node tasks/build.js && rollup -c", "clean": "rimraf es lib scss/generated src/generated", From 69d79efd1cbcb4f2daa186889ade576d0fc6cb00 Mon Sep 17 00:00:00 2001 From: emyarod Date: Mon, 1 Mar 2021 16:42:27 -0600 Subject: [PATCH 4/4] fix(Tabs): prevent tablist role from stealing focus (#7905) * fix(tabs): remove default tablist outlines * fix(Tabs): prevent tablist role from stealing focus Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- packages/components/src/components/tabs/_tabs.scss | 1 + packages/react/src/components/Tabs/Tabs.js | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/components/src/components/tabs/_tabs.scss b/packages/components/src/components/tabs/_tabs.scss index fd661c1a1863..c881bf4532ea 100644 --- a/packages/components/src/components/tabs/_tabs.scss +++ b/packages/components/src/components/tabs/_tabs.scss @@ -453,6 +453,7 @@ padding: 0; overflow: auto hidden; list-style: none; + outline: 0; transition: max-height $duration--fast-01 motion(standard, productive); // hide scrollbars diff --git a/packages/react/src/components/Tabs/Tabs.js b/packages/react/src/components/Tabs/Tabs.js index 4548c0cacf79..bd259a91b4f6 100644 --- a/packages/react/src/components/Tabs/Tabs.js +++ b/packages/react/src/components/Tabs/Tabs.js @@ -274,6 +274,7 @@ export default class Tabs extends React.Component { ) { const currentScrollLeft = this.state.tablistScrollLeft; tab?.tabAnchor?.scrollIntoView({ block: 'nearest', inline: 'nearest' }); + tab?.tabAnchor?.focus(); const newScrollLeft = this.tablist.current.scrollLeft; if (newScrollLeft > currentScrollLeft) { this.tablist.current.scrollLeft += this.OVERFLOW_BUTTON_OFFSET;