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

refactor(dropdowns): use transient props + snapshot testing [DO NOT MERGE] #1962

Closed
wants to merge 4 commits into from

Conversation

ze-flo
Copy link
Contributor

@ze-flo ze-flo commented Oct 24, 2024

⚠️ DO NOT MERGE! ⚠️

Description

This PR adds snapshot tests for our buttons, forms, dropdowns.legacy, and dropdown stories. These tests capture the current visual appearance of the stories before making changes to transient props.

This helps ensure that the refactoring work in #1961 doesn't accidentally introduce any visual bugs. Identical snapshots taken before and after the changes should give reviewers more confidence that no regressions were introduced.

@ze-flo
Copy link
Contributor Author

ze-flo commented Oct 24, 2024

The changes fixed a misalignment of the MenuItem icon when isCompact is true.
Screenshot 2024-10-23 at 12 03 31 PM

The top offset should be 6px.

https://github.com/zendeskgarden/react-components/actions/runs/11490069603/job/31980284923?pr=1962#step:6:1288

FAIL packages/dropdowns/demo/stories/MenuStory.spec.tsx
  ● MenuStory Component › renders MenuStory with compact styling, an arrow, and expanded state

    expect(received).toMatchSnapshot()

    Snapshot name: `MenuStory Component renders MenuStory with compact styling, an arrow, and expanded state 1`

    - Snapshot  - 1
    + Received  + 1

    @@ -416,11 +416,11 @@

      .c14 {
        position: absolute;
        -webkit-transition: opacity 0.1s ease-in-out;
        transition: opacity 0.1s ease-in-out;
    -   top: 10px;
    +   top: 6px;
        left: 12px;
        width: 16px;
        height: 16px;
        opacity: 0;
        color: #1f73b7;

       7 | const renderAndMatchSnapshot = (props: any) => {
       8 |   const { container } = render(<MenuStory {...props} />);
    >  9 |   expect(container.firstChild).toMatchSnapshot();
         |                                ^
      10 | };
      11 |
      12 | const mockItems: Items = [

      at toMatchSnapshot (packages/dropdowns/demo/stories/MenuStory.spec.tsx:9:32)
      at Object.renderAndMatchSnapshot (packages/dropdowns/demo/stories/MenuStory.spec.tsx:88:5)

  ● MenuStory Component › renders MenuStory with all custom props

    expect(received).toMatchSnapshot()

    Snapshot name: `MenuStory Component renders MenuStory with all custom props 1`

    - Snapshot  - 1
    + Received  + 1

    @@ -416,11 +416,11 @@

      .c14 {
        position: absolute;
        -webkit-transition: opacity 0.1s ease-in-out;
        transition: opacity 0.1s ease-in-out;
    -   top: 10px;
    +   top: 6px;
        left: 12px;
        width: 16px;
        height: 16px;
        opacity: 0;
        color: #1f73b7;

I will update the snapshots and use them as reference.

@coveralls
Copy link

Coverage Status

coverage: 95.891%. remained the same
when pulling ae9e4f2 on ze-flo/transient-props-dropdowns-with-specs
into 5a6c53c on main.

@ze-flo
Copy link
Contributor Author

ze-flo commented Oct 24, 2024

Closing since #1961 merged.

@ze-flo ze-flo closed this Oct 24, 2024
@ze-flo ze-flo deleted the ze-flo/transient-props-dropdowns-with-specs branch December 12, 2024 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants