Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Datagrid release review: Row height settings #2720

Closed
17 of 30 tasks
elycheea opened this issue Mar 3, 2023 · 1 comment
Closed
17 of 30 tasks

Datagrid release review: Row height settings #2720

elycheea opened this issue Mar 3, 2023 · 1 comment
Assignees
Labels

Comments

@elycheea
Copy link
Contributor

elycheea commented Mar 3, 2023

Review for release

Readiness

  • One or more scenarios for a design pattern have been identified as a
    useful unit of functionality to publish.
  • A functioning component or components delivering those scenarios have been
    delivered and merged to main.
  • Design maintainer has approved the implementation for those scenarios (via
    a comment on the relevant issue/epic). Datagrid design review: Row height settings #2503

Engineering review

  • Breaking changes have only been introduced with prior approval, discussion
    and documented in release notes. Ideally deprecation notices in the prior
    major version must have been added in a timely fashion.
  • The implementation takes into account, and does not impair, remaining and
    anticipated design scenarios.
  • Public component features (names, props, etc) are consistent with
    Carbon-defined conventions and are consistent internally. Where there
    isn't an existing convention to apply, ensure robust precedents are being
    established.
  • The UI produced is accessible, responsive, translatable, cross-browser,
    and responds to the currently set Carbon theme.
  • Components are functional components using hooks.
  • Public components which produce DOM structures support className.
  • Public components support a ref (via React.forwardRef).
  • Public component supports a Devtools attribute
  • All significant DOM elements have meaningful classes.
  • Additional attributes that are not identified as props (such as title,
    aria-*, etc) are passed through to an appropriate DOM node of the
    component as HTML attributes.
  • No warnings, errors or log messages in the console.
  • Each public component JS is exported in /src/components/index.js, each
    public component SCSS is included in /src/components/_index.scss, and
    each public component has a flag in package-settings.js.
  • Each public component SCSS lists all of the Carbon and C&CS components
    imported and used by the JavaScript code and explicitly imports the SCSS
    for each of these components.

Standards

  • No linter warnings or errors are produced.
  • Carbon tokens (theme, layout, motion) are used unless the design specifies
    otherwise.
  • All components utilizing motion must include reduced-motion queries for
    accessibility purposes - read more here.
  • Code is formatted according to prettier rules (no use of
    //prettier-ignore).
  • Code is clear, maintainable and follows standard React practices and the
    code guidelines.
  • Copyright header in every source file (js, css, scss etc.) with the
    appropriate years.

Testing

  • There is a set of test cases for the components.
  • The tests exercise all inputs (props, slots, etc) and verify appropriate
    outputs.
  • The tests validate the behaviors and interactions defined in the design
    where practical.
  • The tests achieve 100% coverage. Usage of istanbul ignore is appropriate
    and not extensive.
  • No warnings, errors or log messages in the test output.

Documentation

  • Source code is satisfactorily commented and provides DocGen comments for
    all public components and their props.
  • There is a story for each design scenario. The stories expose all relevant
    props and actions, and additional usage documentation and code samples are
    available on the 'Docs' tab along with the props table.
  • There is a sandbox (or multiple sandboxes if appropriate) on CodeSandbox
    for the components.
@matthewgallo
Copy link
Member

matthewgallo commented Apr 3, 2023

Items requiring further review:

  • Min width of flyout seems to still be an isssue Saw note that as long as size labels do not wrap, the current implementation is ok.
  • Double check the focus/tabbing behavior of the row size menu, does the menu need to close when tab is pressed?
  • Still needs sandbox that includes row height settings example
  • "Default" wording still needs to be removed, it is in the default props, see here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants