The ebayui-core-react from ebay

Accessibility: EbaySectionNotice section not being announced by screen readers

EbaySectionNotice section region (role="region") is linking to a description empty element( id="section-notice-attention" element only contains a );

The expected behavior is: "The region has an accessible name that it announced when the region is focused." - while currently the region is not announced by screen readers.

BreadCrumbs return same data twice

Original issue: https://github.corp.ebay.com/React/ebayui-core-react/issues/287

ebayui-core-react/src/ebay-breadcrumbs/breadcrumbs.tsx

Line 50 in 3b3903f

onClick: event => onSelect(event, event.target)

currently there is no way to override onClick on items since it is cloned .

something like this would be helpful
onClick: event => onSelect(event, index)

we can always deconstruct target from event like below

onSelect={({target})=>console.log(target)}

Optional but would be nice if we utilize Storybook action addon which we already use on some components. This is quite handy to do a quick manual test on storybook, making sure important handlers are called.

Original issue: https://github.corp.ebay.com/React/ebayui-core-react/issues/358

Optional but would be nice if we utilize Storybook action addon which we already use on some components. This is quite handy to do a quick manual test on storybook, making sure important handlers are called.

// add import
import { action } from '../../../.storybook/action'

export const _Default = () => {
    // add 2 handlers with action
    const handleConfirm = () => {
        action('onConfirm')();
        close();
    }
    const handleReject = () => {
        action('onReject')();
        close();
    }

    return (
        <div>
          { /* ... */ }
          { /* Add handlers to component, one can see the action name on Storybook addons panel */ }  
        <EbayConfirmDialog 
            open={open}
            onConfirm={handleConfirm}
            onReject={handleReject}
        >
              { /* ... */ }  
        </EbayConfirmDialog>
    </div>
)};

Originally posted by @isuhardi in https://github.corp.ebay.com/React/ebayui-core-react/pull/351#discussion_r2149916

EbayCheckbox: Improve doc, state dependency on EbaySvg

Original issue: https://github.corp.ebay.com/React/ebayui-core-react/issues/345

As a follow up from issue https://github.corp.ebay.com/React/ebayui-core-react/issues/344, we need to improve the EbayCheckbox documentation to clearly state dependency on EbaySVG (user need to load EbaySvg on the page).

We can do it just like the statement on EbayRadio (see https://github.corp.ebay.com/React/ebayui-core-react/tree/master/src/ebay-radio#icons)

ebay-select: TypeScript compilation error with onChange

When EbaySelect with onChange used with TypeScript, I got compilation error due to conflicting types:

error TS2322: Type '(e: ChangeEvent<HTMLSelectElement>, selectedIndex: number, newValue: string | number | readonly string[]) => void' is not assignable to type 'ChangeEventHandler<HTMLSelectElement> & ((e: ChangeEvent<HTMLSelectElement>, selectedIndex: number, newValue: SelectValue) => void)'.
  Type '(e: ChangeEvent<HTMLSelectElement>, selectedIndex: number, newValue: string | number | readonly string[]) => void' is not assignable to type 'ChangeEventHandler<HTMLSelectElement>'.

59         onChange={handleSelectChange}
           ~~~~~~~~

  node_modules/@types/react/index.d.ts:2375:9
    2375         onChange?: ChangeEventHandler<T> | undefined;
                 ~~~~~~~~
    The expected type comes from property 'onChange' which is declared here on type 'IntrinsicAttributes & ClassAttributes<HTMLSelectElement> & SelectHTMLAttributes<HTMLSelectElement> & { ...; } & { ...; }'

How to reproduce

Use any TypeScript (tested with 3.9 and 4.6) with the latest type definitions for react v16.14.30.
Add EbaySelect with onChange

const handleSelectChange: (
  e: React.ChangeEvent<HTMLSelectElement>,
  selectedIndex: number,
  newValue: string | ReadonlyArray<string> | number
) => void = (event, selectedIndex, newValue) => {
  // ....
}

// ...
<EbaySelect
  name="endReason"
  onChange={handleSelectChange}
>
  {benefits.map((benefit, index) => 
    <EbaySelectOption key={index} value={benefit.value}>
      {benefit.title}
    </EbaySelectOption>
  )}
</EbaySelect>

Cause

This is due to conflicting types between:

HTML select onChange with type ChangeEventHandler<HTMLSelectElement> (1 parameter)
EbaySelect onChange with type (e: ChangeEvent<HTMLSelectElement>, selectedIndex: number, newValue: string | number | readonly string[]) => void (3 parameters)

EbaySelect used all select props and try to overwrite its onChange:

ebayui-core-react/src/ebay-select/ebay-select.tsx

Lines 11 to 18 in e13c4e6

    
           export type EbaySelectProps = ComponentProps<'select'> & { 
        
               borderless?: boolean; 
        
               defaultValue?: SelectValue; 
        
               onChange?: (e: ChangeEvent<HTMLSelectElement>, selectedIndex: number, newValue: SelectValue) => void; 
        
               floatingLabel?: string; 
        
               inputSize?: 'default' | 'large'; 
        
               invalid?: boolean; 
        
           };

We should omit onchange from EbaySelect before overwriting it. See https://react-typescript-cheatsheet.netlify.app/docs/advanced/patterns_by_usecase/#props-omit-prop-from-a-type

Workaround

Cast the handler with ChangeEventHandler:

<EbaySelect
  name="endReason"
  onChange={handleSelectChange as React.ChangeEventHandler<HTMLSelectElement>}
>

ebay-tab: Accessibility bug due to duplicated IDs when a page has more than 1 tab

Problems

When there are more than 1 ebay-tabs on a page, the page will get accessibility error (via Chrome lighthouse) due to duplicated ebay-tab IDs.
Unfortunately, there is no workaround for this issue

How to Reproduce

Add 2 or more EbayTabs inside a page. You can create a storybook.
Check on Chrome LIghthouse or other accessibility checker. You will get an error like ARIA IDs are not unique (see https://web.dev/duplicate-id-aria/?utm_source=lighthouse&utm_medium=devtools)

Cause

This is due to hardcoded id on nesting tab, which will always have semi-dynamic id default-tab-{child-tab-index}:

ebayui-core-react/src/ebay-tabs/tab.tsx

Lines 31 to 42 in 26ee62e

    
           <div 
        
               {...rest} 
        
               ref={refCallback} 
        
               aria-controls={`default-tabpanel-${index}`} 
        
               aria-selected={selected} 
        
               className={classNames(className, 'tabs__item')} 
        
               id={`default-tab-${index}`} 
        
               role="tab" 
        
               tabIndex={selected ? 0 : -1} 
        
               onClick={onClick} 
        
               onKeyDown={onKeyDown} 
        
           >

Possible Fix

Use prefix ID for nested tab id instead of just default-tab-${index}. We can get prefix id from the EbayTab id (if provided by consumer). This pattern is used on other components like dialogs or breadcrumbs.

Add Contributing.md

Add a separated CONTRIBUTING.md, which is common practice for having contribution guideline on public GitHub (see https://docs.github.com/en/communities/setting-up-your-project-for-healthy-contributions/setting-guidelines-for-repository-contributors)

Split contributions from README
We should state in details what we want to have for when people contribute to the project:
- linting & testing before PR
- Updating component readme in case of changing its API
- Commit message conventions
- and more...
- For an example, we can have a look at eBayUI Core Marko contibution.md https://github.com/eBay/ebayui-core/blob/master/CONTRIBUTING.md

@darkwebdev @HenriqueLimas We can use this issue to discuss what we want to have

Use React v16 on both dev and peer dependencies

Problem

Currently, eBayUI Core React only support React v16 via its peer dependency:

ebayui-core-react/package.json

Lines 43 to 47 in e914482

    
           "peerDependencies": { 
        
             "@ebay/skin": "^14.0.0", 
        
             "react": "^16.0.0", 
        
             "react-dom": "^16.0.0" 
        
           },

However, on its devDependencies , we use v18 for react, types/react, react-dom, and types/react-dom. I think this will cause problem since eBayUI Core React will be compiled using type definitions for React v18, while the consumer is still on v16. There might be new React attributes or props on the compiled type definitions.

Proposed Solution

One options is to use React v16 for both peer and dev dependencies, for now. When we upgrade support to React v18, then we will update both peer and dev dependencies to use v18.

Cleanup: Removed all deprecated features on next major release

We should removed all deprecated components, props, and enums on the next major release, maybe on the next Skin upgrade.

Besides removing, we must document it on the release note. For example, deprecating EbayDialog in favour of EbayFullscreenDialog, EbayLightboxDialog, and so on.

List

The following is the features to be removed. Feel free to add more so we can remove the following in one go.

Components

EbayDialog. Replaced by EbayFullscreenDialog, EbayLightboxDialog, EbayPanelDialog, and EbayDrawerDialog
EbayFloatingLabel. Replaced by using EbayTextbox or EbaySelect with propfloatingLabel.
EbayLegacyFloatingLabel.
EbayNoticeContent in src/ebay-notice-content. Replace by importing EbayNoticeContent from component ebay-notice-base, ebay-page-notice, ebay-section-notice, or ebay-inline-notice.
EbayTab. Replaced by EbayTabs.

Component Props

EbayFullscreenDialog prop sliding.
EbayButton priority="delete". Replaced by EbayButton prop variant ="destructive".
EbayTabs fake.
EbayTextbox underline
EbayProgressStep number. Replaced by property state.

Component Constants and Enums

From tooltip-utils constants-legacy TOOLTIP_TYPES and POINTER_TYPES. Replaced by TooltipType union type.
EbayField fieldLayoutType, fieldDescriptionType, and fieldDescriptionPosition. Replaced by union types FieldLayoutType, FieldDescriptionType, and FieldDescriptionPosition.
EbayInfotip enums variants. Replaced by Variant union type.
EbayTab activations and sizes. Replaced by EbayTabs.

EbayFakeMenuButton

Marko version:
https://ebay.github.io/ebayui-core/?path=/story/ebay-fake-menu-button--basic

Introduce prettier and split linting and code styling

Introduce prettier for code styling formatting. This is not just introducing prettier to our dependency and build process but more than that!

Splitting code stye formatting (prettier) with linting for code quality rules (eslint)

The idea that we should not care and waste our time discussing about code styling and delegate/automate that matters to prettier. This will free us more time and energy to discuss more important stuff. See https://www.morling.dev/blog/the-code-review-pyramid/ about this common phenomenon on code review

This means that our eslint rules should focus on code quality and contains rules to prevent bugs (e.g no unused vars, no implivit globals, prefer const, ...).
Any code styling rules (e.g trailing coma, space, quotes, white space) should be handled by prettier.
For motivation on splitting linting and code styling, see https://prettier.io/docs/en/comparison.html

Preferable to use the default prettier config as-is

Again, this is to delegate anything related to code styling to prettier. As soon as we add prettier config, even if it's just a small config, it might start discussions about 2-vs-4 spaces, single vs double quotes, ...

People will then focus and invested on discussions around mundane aspects like code style formatting, whereas more important aspects (does the code change do what it is supposed to do, is it performant, is it backwards-compatible for existing API, do we write good tests, and many others) tend to get less attention. See https://www.morling.dev/blog/the-code-review-pyramid/

Drop eslint-config-ebay

eslint-config-ebay is based on airbnb eslint rules which is a mix of code styling (and very opinionated about it) and bug-prevention rules. Unfortunately the eBay rule is not maintained since 2019, some rules might outdated, not in-synch with AirBnB anymore, and does not consider for the latest ES6+ (e.g ES2022) and TypeScript.

Would drop eslint-config-ebay and stick with recommended eslint rules (e.g eslint:recommended, @typescript-eslint/eslint-recommended, react/recommended) and add more rules to help us catching bugs (e.g react-hooks/recommended)

Others

Include prettier as-part of husky or lint-staged

ebay-radio: TypeScript compilation error with onChange

Similar to issue #75, I got TypeScript compilation error when using EbayCheckbox with onChange:

error TS2322: Type '(e: ChangeEvent<HTMLInputElement>, value: string | number, checked: boolean) => void' is not assignable to type 'ChangeEventHandler<HTMLInputElement> & ((e: ChangeEvent<HTMLInputElement>, value: string | number, checked: boolean) => void)'.
  Type '(e: ChangeEvent<HTMLInputElement>, value: string | number, checked: boolean) => void' is not assignable to type 'ChangeEventHandler<HTMLInputElement>'.

68       <EbayCheckbox checked value="123" id="checkbox-21" onChange={handleCheckboxChange}>
                                                            ~~~~~~~~

  node_modules/@types/react/index.d.ts:2221:9
    2221         onChange?: ChangeEventHandler<T> | undefined;
                 ~~~~~~~~
    The expected type comes from property 'onChange' which is declared here on type 'IntrinsicAttributes & InputProps & EbayCheckboxProps & { children?: ReactNode; }'

How to reproduce

Use any TypeScript (tested with 3.9 and 4.6) with the latest type definitions for react v16.14.30.
Add EbayCheckbox with onChange

const handleCheckboxChange: (
  e: React.ChangeEvent<HTMLInputElement>, value: string | number, checked: boolean
) => void = (event, selectedIndex, newValue) => {
   // ...
}

// ...
<EbayCheckbox checked value="123" id="checkbox-21" onChange={handleCheckboxChange}>
  <EbayLabel>Default</EbayLabel>
</EbayCheckbox>

Cause

This is due to conflicting types between:

HTML input onChange with type ChangeEventHandler<HTMLInputElement> (1 parameter)
EbaySelect onChange with type (e: ChangeEvent<HTMLInputElement>, value: string | number, checked: boolean) => void (3 parameters)

EbayCheckbox used all <input> props and try to overwrite its onChange:

ebayui-core-react/src/ebay-checkbox/checkbox.tsx

Lines 8 to 12 in e13c4e6

    
           type InputProps = Omit<ComponentProps<'input'>, 'size'> 
        
           type EbayCheckboxProps = { 
        
               size?: Size; 
        
               onChange?: (e: ChangeEvent<HTMLInputElement>, value: string | number, checked: boolean) => void; 
        
           }

We should omit onchange from EbayCheckbox before overwriting it. See https://react-typescript-cheatsheet.netlify.app/docs/advanced/patterns_by_usecase/#props-omit-prop-from-a-type

Workaround

Cast the handler with ChangeEventHandler:

<EbayCheckbox
  onChange={handleCheckboxChange as React.ChangeEventHandler<HTMLInputElement>}
>

EbayFilter

Marko Source, Storybook

Original eBayUI API compatibility:

DOM structure of the component is preserved
All original attributes are supported
All original events are triggered

Documentation & tests:

Readme
Rendering tests (snapshots/manual)
Interaction tests (clicking, etc)
Storybook has examples with multiple use-cases

Bug: Storyshots did not create test snapshot for all storybook

Problems

While modifying the ebay-button, I realized that the StoryShots did not create test snapshot for all Storybook stories.

Here's one of the ebay-button story. It contains 2 buttons:

ebayui-core-react/src/ebay-button/__tests__/index.stories.tsx

Lines 8 to 16 in e3ca8b1

    
           .add(`Default`, () => ( 
        
               <> 
        
                   <p><EbayButton 
        
                       onClick={action(`clicked`)} 
        
                       onEscape={action('escape pressed')} 
        
                   >Hello, I am a button!</EbayButton></p> 
        
                   <p><EbayButton href="https://ebay.com">Hello, I am a link!</EbayButton></p> 
        
               </> 
        
           ))

Meanwhile, the snapshot from that story only containing 1 button:

ebayui-core-react/src/ebay-button/__tests__/__snapshots__/index.spec.tsx.snap

Lines 3 to 11 in e3ca8b1

    
           exports[`Storyshots ebay-button Default 1`] = ` 
        
           <p> 
        
             <button 
        
               class="btn btn--secondary" 
        
             > 
        
               Hello, I am a button! 
        
             </button> 
        
           </p> 
        
           `;

The same with other longer story:

ebayui-core-react/src/ebay-button/__tests__/index.stories.tsx

Lines 25 to 38 in e3ca8b1

    
           .add(`Priority`, () => ( 
        
               <> 
        
                   <p><EbayButton priority="primary">Primary Button</EbayButton></p> 
        
                   <p><EbayButton priority="primary" href="https://ebay.com">Primary Link</EbayButton></p> 
        
                   <p><EbayButton priority="secondary">Secondary Button</EbayButton></p> 
        
                   <p><EbayButton priority="secondary" href="https://ebay.com">Secondary Link</EbayButton></p> 
        
                   <p><EbayButton>Secondary Button (Default)</EbayButton></p> 
        
                   <p><EbayButton href="https://ebay.com">Secondary Link (Default)</EbayButton></p> 
        
                   <p><EbayButton priority="tertiary">Tertiary Button</EbayButton></p> 
        
                   <p><EbayButton priority="tertiary" href="https://ebay.com">Tertiary Link</EbayButton></p> 
        
                   <p><EbayButton priority="delete">Delete Button</EbayButton></p> 
        
                   <p><EbayButton priority="none">Base Button</EbayButton></p> 
        
               </> 
        
           ))

The snapshot only capture the first button:

ebayui-core-react/src/ebay-button/__tests__/__snapshots__/index.spec.tsx.snap

Lines 131 to 139 in e3ca8b1

    
           exports[`Storyshots ebay-button Priority 1`] = ` 
        
           <p> 
        
             <button 
        
               class="btn btn--primary" 
        
             > 
        
               Primary Button 
        
             </button> 
        
           </p> 
        
           `;

This is a big problem since our test coverage might be lower than we thought.

Cause

However, this problem only existed if we wrap a Storybook story with React Fragment. If we wrap it with a <div> of <p>, the Storyshots capture the whole story.

For an example, this story is wrapped by a <div>:

ebayui-core-react/src/ebay-button/__tests__/index.stories.tsx

Lines 62 to 69 in e3ca8b1

    
           .add(`Transparent`, () => ( 
        
               <div style={{ background: 'rgba(66, 214, 205, 0.5)' }}> 
        
                   <p><EbayButton>Default Button</EbayButton></p> 
        
                   <p><EbayButton transparent>Transparent Button</EbayButton></p> 
        
                   <p><EbayButton transparent priority="delete">Transparent Delete Button</EbayButton></p> 
        
                   <p><EbayButton href="https://ebay.com" transparent>Transparent Link</EbayButton></p> 
        
               </div> 
        
           ))

The generated snapshot contains all buttons:

ebayui-core-react/src/ebay-button/__tests__/__snapshots__/index.spec.tsx.snap

Lines 151 to 185 in e3ca8b1

    
           exports[`Storyshots ebay-button Transparent 1`] = ` 
        
           <div 
        
             style="background: rgba(66, 214, 205, 0.5);" 
        
           > 
        
             <p> 
        
               <button 
        
                 class="btn btn--secondary" 
        
               > 
        
                 Default Button 
        
               </button> 
        
             </p> 
        
             <p> 
        
               <button 
        
                 class="btn btn--secondary btn--transparent" 
        
               > 
        
                 Transparent Button 
        
               </button> 
        
             </p> 
        
             <p> 
        
               <button 
        
                 class="btn btn--delete btn--transparent" 
        
               > 
        
                 Transparent Delete Button 
        
               </button> 
        
             </p> 
        
             <p> 
        
               <a 
        
                 class="fake-btn fake-btn--secondary fake-btn--transparent" 
        
                 href="https://ebay.com" 
        
               > 
        
                 Transparent Link 
        
               </a> 
        
             </p> 
        
           </div> 
        
           `;

I suspect this has thing to do our StoryShot configuration, which only take snapshot from the first child:

ebayui-core-react/config/jest/storyshots.ts

Lines 5 to 8 in e3ca8b1

    
           const reactTestingLibrarySerializer = { 
        
               print: (val, serialize, indent) => serialize(val.container.firstChild), 
        
               test: (val) => val && val.hasOwnProperty("container"), 
        
           };

Options so far:

We might have to take snapshot of the whole container, but it might pollute our snapshot with react-testing-library container
We have to replace all React Fragment on Storybook wrapper with other element like <div>

ebay-menu-button

https://github.com/eBay/ebayui-core/tree/master/src/components/ebay-menu-button

Original eBayUI API compatibility:

DOM structure of the component is preserved
All original attributes are supported
All original events are triggered

Documentation & tests:

Readme
Rendering tests (snapshots/manual)
Interaction tests (clicking, etc)
Storybook has examples with multiple use-cases

Use strict: true for TS compilation

Increase test coverage

I think we need more tests:

EbayCombobox

Marko Source, Storybook

Original eBayUI API compatibility:

DOM structure of the component is preserved
All original attributes are supported
All original events are triggered

Documentation & tests:

Readme
Rendering tests (snapshots/manual)
Interaction tests (clicking, etc)
Storybook has examples with multiple use-cases

Should add invalid background color here

Original issue: https://github.corp.ebay.com/React/ebayui-core-react/issues/393

ebayui-core-react/src/ebay-floating-label/hooks.tsx

Lines 28 to 32 in 3b3903f

    
           // input background in Skin 12, used for autofill detection 
        
           const textboxElementBackgroundRGB = [ 
        
               'rgb(245, 245, 245)', // DEPRECATED 
        
               'rgb(247, 247, 247)' // From skin version 12.6.0 
        
           ]

When field is invalid, the background color isrgb(254, 245, 246). Currently, for invalid floating label or floating select, if value is not set, the label will always be floated, which should be inline.

ebay-page-notice: Update icon size based on Skin v14.12.0

Skin v14.12.0 contains changes on Page Notice which make the icon smaller and some adjusted spacing.

Since the page notice icon is rendered by our ebay-icon, we need to change our implementation to use the right icon sizes:

ebayui-core-react/src/ebay-page-notice/page-notice.tsx

Lines 31 to 37 in e13c4e6

    
           <div className="page-notice__header" id={`${status}-status`}> 
        
               <EbayIcon 
        
                   name={`${status}-filled` as any} 
        
                   a11yText={ariaLabel} 
        
                   a11yVariant="label" 
        
               /> 
        
           </div>

Before:

After:

See related issues:

Skin v14.12.0 release: https://github.com/eBay/skin/releases/tag/v14.2.0
PR on Skin, updating page notice: eBay/skin#1838

add storybook action to tests for new dialogs

Feel free to also add storybook action to test onConfirm is being called, but I think it's not necessary, unlike the confirm dialog

From internal GH:

While adding storybook, maybe also improve unit test. Followup from my comment on improving ebay-confirm-dialog test https://github.corp.ebay.com/React/ebayui-core-react/pull/351#discussion_r2149911:

let wrapper
// Replace with these 2 handlers
let handleConfirm
let handleReject

beforeEach(() => {
    handleConfirm = jest.fn()
    handleReject = jest.fn()

    wrapper = mount(
        <EbayConfirmDialog
            open
            onReject={handleReject}
            onConfirm={handleConfirm}
        >
            { /* ... */ }
        </EbayConfirmDialog>
    )
})

// ...

// add 2 tests
it('should trigger onReject when reject button button is clicked', () => {
    wrapper.find(`button.confirm-dialog__reject`).simulate('click')
    expect(handleReject).toBeCalled()
    expect(handleConfirm).not.toBeCalled()
})

it('should trigger onConfirm when confirm button button is clicked', () => {
    wrapper.find(`button.confirm-dialog__confirm`).simulate('click')
    expect(handleConfirm).toBeCalled()
    expect(handleReject).not.toBeCalled()
})

Accessibility: Infotip focus on collapse

After collapsing an infotip the focus get lost in the page but it should focus the host element

EbayWindowNotice

Would be nice to have:
https://ebay.github.io/skin/archive/v9.5/ds6/index.html#notice

Delete button is not rendered properly with Skin v14

Problem

Since Skin 14 and eBayUI Core v10, the delete button is not rendered properly: the button with red border is now rendered as default button with black border:

Causes

With Skin 14, the delete button is now called destructive buttons. Instead of just 1 kind of button, there are 3 types of destructive buttons:

The CSS modifier has changed from btn--delete to btn--destructive.
Apart from CSS class btn--destructive, we need to add btn--primary, btn--secondary, or btn--tertiary in order to render the button correctly.

EbayToast(Drawer) Dialog: snapshots contain only open button, but not the dialog itself

(migrated from internal repo)

"we can configure storyshots to cover that, @hlimas knows how to"

a11yCloseText in EbayFullscreenDialog doesn't appear to see the a11y text

ebay-star-rating and ebay-star-rating-select

Description

New component based on eBay/skin#1725

https://github.com/eBay/ebayui-core/tree/master/src/components/ebay-star-rating
and
https://github.com/eBay/ebayui-core/tree/master/src/components/ebay-star-rating-select

Original eBayUI API compatibility:

DOM structure of the component is preserved
All original attributes are supported
All original events are triggered

Documentation & tests:

Readme
Rendering tests (snapshots/manual)
Interaction tests (clicking, etc)
Storybook has examples with multiple use-cases

Original eBayUI API compatibility:

DOM structure of the component is preserved
All original attributes are supported
All original events are triggered

Documentation & tests:

Readme
Rendering tests (snapshots/manual)
Interaction tests (clicking, etc)
Storybook has examples with multiple use-cases

EbaySectionNotice section not being announced by screen readers

Original issue: https://github.corp.ebay.com/React/ebayui-core-react/issues/397

EbaySectionNotice section region (role="region") is linking to a description empty element( id="section-notice-attention" element only contains a <svg>);

Following the solution reported in https://jirap.corp.ebay.com/browse/APOLLO-3398 and https://jirap.corp.ebay.com/browse/APOLLO-3395, the expected behavior is: "The region has an accessible name that it announced when the region is focused." - while currently the region is not announced by screen readers.

Issue created on ebayui-app-react: React/ebayui-app-react#152

ebay-cta-buton large: text is misaligned

https://opensource.ebay.com/ebayui-core-react/main/index.html?path=/story/ebay-cta-button--large

bug in skin:

Henrique has created an issue:
eBay/skin#1890

Support Skin 14

We need to upgrade Skin support from 13 to 14

Add eslint plugins & rules for hooks

Original issue: https://github.corp.ebay.com/React/ebayui-core-react/issues/277

Add the official eslint rules for React hooks (see https://reactjs.org/docs/hooks-rules.html and https://www.npmjs.com/package/eslint-plugin-react-hooks) to our eslint config.

This is to prevent hooks-related issues like https://github.corp.ebay.com/React/ebayui-core-react/issues/276 and https://github.corp.ebay.com/React/ebayui-core-react/issues/347, where we got stale props due to not properly stating variables on hook dependency array. This problem is hard to detect and also not easy to test.

Deal with lots of linting warnings

EbayTriStateCheckbox

Skin:

eBayUI Marko:
https://github.com/eBay/ebayui-core/tree/master/src/components/ebay-tri-state-checkbox
https://ebay.github.io/ebayui-core/?path=/story/ebay-tri-state-checkbox--default

README: Document about pass-through attributes feature

Original issue: https://github.corp.ebay.com/React/ebayui-core-react/issues/387

Just like the eBayUI Core Marko principles, eBayUI Core React also supports all valid HTML attributes on all its components (e.g id, aria-*, data-*, tabindex, style, ...) by using pass through feature.

The Marko has documented this feature on its readme (see https://github.com/eBay/ebayui-core#pass-through-attributes) but not on our React part.

Adding this feature to our document will help to solve questions/problems like this on Slack especially for new users, who are not familiar with eBayUI Core Marko and not aware that most eBayUI Core component supports any HTML attributes.

Detect missing variables on hook dependency array

Original issue: https://github.corp.ebay.com/React/ebayui-core-react/issues/374

This is a follow-up from issue #347 , where we got stale props due to not properly stating variables on hook dependency array. This problem is hard to detect and also not easy to test.

One options is to introduce eslint rule eslint-plugin-react-hooks to help us detecting this kind of problems.

Clean-up release and git tags

When renaming the repo to ui-core-react (see PR #57 ), we reset the version back to version 1.0.0

There are currently 80+ release tags from the old releases, when we're still using the old name. These old tags potentially can break our release in the future. The new version might clash with the existing older version so version bumping will failed since the git tag has already existed.

While this version clash will happen somewhen in the future when we reach version 9.13 but it's best if we clean up git tags earlier.

Actions

We can delete the following tags or rename them to [email protected]:

Tags from 9.13.1 to 10.0.17
Tags with prefix 'v', from v10.0.12 to v10.0.10

Upgrade storybook with addons

Original issue: https://github.corp.ebay.com/React/ebayui-core-react/issues/268

@storybook/react should be 6+
@storybook/addon-storyshots should be 6+

documentation: https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#deprecated-configure

Both render and config properties will be deprecated in v6 and removed in v7:

initStoryshots({
    renderer: mount,
    config: ({ configure }) =>
        configure(() => {
            require('./index.stories')
        }, module)
})

possible changes in jest.config.js:

{
transform: {
        '^.+\\.[jt]sx?$': 'ts-jest'
    },
globals: {
        'ts-jest': {
            tsConfig: {
                allowJs: true
            }
}

Replace static id on EbayListboxButton in case of prefixId is defined

Problems

As a follow up from PR #252, which used static id on the button span when the EbayListboxButton has prefixId (see this comment). The use of static id will cause another accessibility issue if we have more than 1 EbayListboxButton on the same page (more than 1 element with the same id).

Marko widget/component has the concept of 'key' where it can automatically generate unique id for an HTML element. React does not have this out-of-the-box (see issue #210).

Solutions

Please replace static id with dynamic id. However, please take in mind that we can't use random id here since it will cause problem when you re-hydrate a server-side-rendered page on browser.

Related Issues

The solution is related to issue #210

Replace `addon-info` with `addon-docs`

addon-info is deprecated in storybook v6

https://medium.com/storybookjs/storybook-docspage-e185bc3622bf
https://github.com/storybookjs/storybook/blob/next/addons/docs

EbaySnackbarDialog

skin:
https://ebay.github.io/skin/#snackbar-dialog

marko:
https://github.com/eBay/ebayui-core/tree/master/src/components/ebay-snackbar-dialog
https://ebay.github.io/ebayui-core/?path=/story/ebay-snackbar-dialog--basic

EbayDrawer: Cannot change expandable at runtime

When EbayDrawer Dialog expandable prop is changed while the dialog being shown, it does not cause the dialog to re-render.

How to reproduce

Create a storybook:

export const _TriggerExpanded = () => {
  const [open, setOpen] = useState(false);
  const [expanded, setExpanded] = useState(false);

  return (
    <>
      <EbayButton onClick={() => setOpen(!open)}>Open Drawer</EbayButton>
      <EbayDrawerDialog open={open} onClose={() => setOpen(false)} expanded={expanded}>
        <p>
          Trigger Dialog Expanded programmatically.
        </p>

        <EbayButton onClick={() => setExpanded(!expanded)} priority="secondary">
          {expanded ? "Collapse Drawer" : "Expand Drawer"}
        </EbayButton>
      </EbayDrawerDialog>
    </>
  );
};

Once the Drawer is displayed, you cannot expand or collapse the dialog when you click the dialog button "Expand Drawer".

Expected

If I create a similar story on https://github.com/eBay/ebayui-core, you can change the drawer dialog expandable while having the dialog shown

<ebay-drawer-dialog
    a11y-close-text='Close drawer'
    aria-labelledby='drawer-title'
    open=state.open
    on-close('closeDrawer')
    on-expanded('emit', 'expanded')
    on-collapsed('emit', 'collapsed')
    on-open('emit', 'open')
    expanded=state.expanded>

    <@header#drawer-title>Heading Text</@header>

    <p>Trigger Dialog Expanded programmatically.</p>

    <@footer>
        <ebay-button on-click('triggerExpand')>
            ${state.expanded ? 'Collapse Drawer' : 'Expand Drawer'}
        </ebay-button>
    </@footer>
</ebay-drawer-dialog>

<ebay-button on-click('openDrawer')>Open drawer</ebay-button>
class {
    onCreate() {
        this.state = { open: false, expanded: false };
    }
    openDrawer() {
        this.state.open = true;
    }
    triggerExpand() {
        this.state.expanded = !this.state.expanded;
    }
    closeDrawer() {
        this.state.open = false;
        this.emit('close', arguments);
    }
}

drawer-expandable.mov

Dialog : aria-live defaulting to polite

Original issue: https://github.corp.ebay.com/React/ebayui-core-react/issues/323

https://github.corp.ebay.com/React/ebayui-core-react/blob/2d233a2f76eb318abfd4514ac859452381b7cd42/src/ebay-dialog-base/components/dialogBase.tsx#L87

ebay-ui-core logic
https://github.com/eBay/ebayui-core/blob/406000378d778c0a62a9edb44aa49cac6869c909/src/components/components/ebay-dialog-base/component.js#L78
complementary code
https://github.corp.ebay.com/React/ebayui-core-react/blob/2d233a2f76eb318abfd4514ac8594[…]52381b7cd42/src/ebay-dialog-base/components/dialogBase.tsx

page scrolls up to the top when the user clicks on listbox-button

Old default
setTimeout(() => optionsContainerRef.current.focus(), 0)
https://user-images.githubusercontent.com/8145951/176749394-85dbf522-9fa4-4a99-9f2e-5c7ec0b90654.mov
May want add preventScroll Param
New
setTimeout(() => optionsContainerRef.current.focus({ preventScroll: true }), 0)
https://user-images.githubusercontent.com/8145951/176749418-cf2c426d-12c1-4597-88c5-af8682ed1749.mov

EbaySvg: allow to include/exclude certain icons

We need to provide some props for including only required icons in the resulting SVG. Another way is to include all icons (like it works now) but exclude some that are not needed.

Example:

type Props = {
  include?: Icon[];
  exclude?: Icon[];
}
<EbaySvg include={['calendar', 'chevronUp', 'chevronDown']} />
<EbaySvg exclude={['calendar', 'chevronUp', 'chevronDown']} />

Small nuance:
EbaySvg component is generated by scripts/update-icons.js

Contributing document

For a good open source project (public or internal), it would be nice to have a guideline on how to contribute to this project. Things should be cover on the document:

System requirements
Branching strategy
PR requirements:
- Unit tests
- Storybook
- Update component readme
- TypeScript type definitions
- Pass the build (lint, unit test, ...)
Defintion of Done:
- Comply to DS6, Skin, Playbook
- Code review
Release process:
- Versioning
- Breaking changes
- Steps to release
- Update Storybook

For start, we can have similar contributing guideline as the one from eBay UI Core Marko (https://github.com/eBay/ebayui-core/blob/master/CONTRIBUTING.md)

Extend test for EbayDialog

As a follow up for bugfix on aria-hidden: https://github.corp.ebay.com/React/ebayui-core-react/pull/120#issuecomment-1329665

The test should test if the aria-hidden is set accordingly in case of dialog is open and closed. Possible test case:

mount dialog with other divs

mount(
  <>
    <div id="test-1"/>
    <div id="test-2"/>
    <EbayDialog>...</EbayDialog>
    <div id="test-3"/>
  </>
)

Assert no aria-hidden on test divs
trigger dialog to show
Assert aria-hidden=true on test divs
trigger dialog to close
Assert no aria-hidden on test divs

ebay-carousel

Original implementation

Original eBayUI API compatibility:

DOM structure of the component is preserved
All original attributes are supported
All original events are triggered

Documentation & tests:

Readme
Rendering tests (snapshots/manual)
Interaction tests (clicking, etc)
Storybook has examples with multiple use-cases

	export type EbaySelectProps = ComponentProps<'select'> & {
	borderless?: boolean;
	defaultValue?: SelectValue;
	onChange?: (e: ChangeEvent<HTMLSelectElement>, selectedIndex: number, newValue: SelectValue) => void;
	floatingLabel?: string;
	inputSize?: 'default' \| 'large';
	invalid?: boolean;
	};

	<div
	{...rest}
	ref={refCallback}
	aria-controls={`default-tabpanel-${index}`}
	aria-selected={selected}
	className={classNames(className, 'tabs__item')}
	id={`default-tab-${index}`}
	role="tab"
	tabIndex={selected ? 0 : -1}
	onClick={onClick}
	onKeyDown={onKeyDown}
	>

	"peerDependencies": {
	"@ebay/skin": "^14.0.0",
	"react": "^16.0.0",
	"react-dom": "^16.0.0"
	},

	type InputProps = Omit<ComponentProps<'input'>, 'size'>
	type EbayCheckboxProps = {
	size?: Size;
	onChange?: (e: ChangeEvent<HTMLInputElement>, value: string \| number, checked: boolean) => void;
	}

	.add(`Default`, () => (
	<>
	<p><EbayButton
	onClick={action(`clicked`)}
	onEscape={action('escape pressed')}
	>Hello, I am a button!</EbayButton></p>
	<p><EbayButton href="https://ebay.com">Hello, I am a link!</EbayButton></p>
	</>
	))

	exports[`Storyshots ebay-button Default 1`] = `
	<p>
	<button
	class="btn btn--secondary"
	>
	Hello, I am a button!
	</button>
	</p>
	`;

	.add(`Priority`, () => (
	<>
	<p><EbayButton priority="primary">Primary Button</EbayButton></p>
	<p><EbayButton priority="primary" href="https://ebay.com">Primary Link</EbayButton></p>
	<p><EbayButton priority="secondary">Secondary Button</EbayButton></p>
	<p><EbayButton priority="secondary" href="https://ebay.com">Secondary Link</EbayButton></p>
	<p><EbayButton>Secondary Button (Default)</EbayButton></p>
	<p><EbayButton href="https://ebay.com">Secondary Link (Default)</EbayButton></p>
	<p><EbayButton priority="tertiary">Tertiary Button</EbayButton></p>
	<p><EbayButton priority="tertiary" href="https://ebay.com">Tertiary Link</EbayButton></p>
	<p><EbayButton priority="delete">Delete Button</EbayButton></p>
	<p><EbayButton priority="none">Base Button</EbayButton></p>
	</>
	))

	exports[`Storyshots ebay-button Priority 1`] = `
	<p>
	<button
	class="btn btn--primary"
	>
	Primary Button
	</button>
	</p>
	`;

	.add(`Transparent`, () => (
	<div style={{ background: 'rgba(66, 214, 205, 0.5)' }}>
	<p><EbayButton>Default Button</EbayButton></p>
	<p><EbayButton transparent>Transparent Button</EbayButton></p>
	<p><EbayButton transparent priority="delete">Transparent Delete Button</EbayButton></p>
	<p><EbayButton href="https://ebay.com" transparent>Transparent Link</EbayButton></p>
	</div>
	))

	exports[`Storyshots ebay-button Transparent 1`] = `
	<div
	style="background: rgba(66, 214, 205, 0.5);"
	>
	<p>
	<button
	class="btn btn--secondary"
	>
	Default Button
	</button>
	</p>
	<p>
	<button
	class="btn btn--secondary btn--transparent"
	>
	Transparent Button
	</button>
	</p>
	<p>
	<button
	class="btn btn--delete btn--transparent"
	>
	Transparent Delete Button
	</button>
	</p>
	<p>
	<a
	class="fake-btn fake-btn--secondary fake-btn--transparent"
	href="https://ebay.com"
	>
	Transparent Link
	</a>
	</p>
	</div>
	`;

	const reactTestingLibrarySerializer = {
	print: (val, serialize, indent) => serialize(val.container.firstChild),
	test: (val) => val && val.hasOwnProperty("container"),
	};

	// input background in Skin 12, used for autofill detection
	const textboxElementBackgroundRGB = [
	'rgb(245, 245, 245)', // DEPRECATED
	'rgb(247, 247, 247)' // From skin version 12.6.0
	]

	<div className="page-notice__header" id={`${status}-status`}>
	<EbayIcon
	name={`${status}-filled` as any}
	a11yText={ariaLabel}
	a11yVariant="label"
	/>
	</div>

ebay / ebayui-core-react Goto Github PK

ebayui-core-react's Introduction

ebayui-core-react

Requirements

eBayUI Components

Getting Started

for quick development/POC

for smaller bundle size

Notes

Pass-Through Attributes

Issues

Changelog

version 5.x (Skin 16, breaking changes in event callbacks)

version 4.x (Skin 16, breaking changes in icon names)

version 3.x (Skin 15, some breaking changes in dialog components)

version 2.x (Skin 15)

version 10.x (Skin 14)

version 9.x (skin 13)

version 8.x (skin 12)

version 6.x (skin 10)

version 5.x (removed less, changed imports to minimize bundle size)

version 3.x (skin 9, react 16.8 with hooks support)

version 2.x (skin 7, react 16.7)

legacy

ebayui-core-react's People

Contributors

Stargazers

Watchers

Forkers

ebayui-core-react's Issues

How to reproduce

Cause

Workaround

Problems

How to Reproduce

Cause

Possible Fix

Problem

Proposed Solution

List

Components

Component Props

Component Constants and Enums

Splitting code stye formatting (prettier) with linting for code quality rules (eslint)

Preferable to use the default prettier config as-is

Drop eslint-config-ebay

Others

How to reproduce

Cause

Workaround

Problems

Cause

Problem

Causes

Description

Actions

Problems

Solutions

Related Issues

How to reproduce

Expected

Recommend Projects

Recommend Topics

Recommend Org