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

[isConformant] Implementing component-handles-classname test #15707

Merged

Conversation

czearing
Copy link
Collaborator

@czearing czearing commented Oct 27, 2020

Pull request checklist

Description of changes

Summary

  1. Adding a component-handles-classname test to isConformant's defaultTests.
  2. Removing redundant tests ComponentConformance.test.tsx from react-internal.
  3. Removing redundant ComponentConformance.test.tsx from react.
  4. Removing redundant tests in react-northstar isConformant tests.
  5. Disabling tests that don't handle classNames correctly. (Either they aren't applied to the root element or not passing classNames correctly. )

component-handles-classname

What this test does:

  1. Checks if the component handles the className prop.
  2. Checks if the component handles any existing default provided classNames correctly.
  3. Checks if the component puts the className on the root element.
  /** Component file handles classname prop */
  'component-handles-classname': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => {
    const { Component, wrapperComponent, helperComponents = [], requiredProps, customMount = mount } = testInfo;

    let el = customMount(<Component {...requiredProps} />);
    let component = getComponent(el, helperComponents, wrapperComponent);

    const defaultClassNames =
      component
        ?.getDOMNode()
        ?.getAttribute('class')
        ?.split(' ') || [];

    const testClassName = 'testComponentClassName';
    const mergedProps: Partial<{}> = {
      ...requiredProps,
      className: 'testComponentClassName',
    };

    el = customMount(<Component {...mergedProps} />);
    component = getComponent(el, helperComponents, wrapperComponent);

    // eslint-disable-next-line @typescript-eslint/no-explicit-any
    const classNames: any = component
      .getDOMNode()
      .getAttribute('class')
      ?.split(' ');

    it(`handles className prop`, () => {
      expect(classNames.indexOf(testClassName) >= 0).toEqual(true);
    });

    it(`handles component's default classNames`, () => {
      if (defaultClassNames.length && defaultClassNames[0] !== '') {
        for (const defaultClassName of defaultClassNames) {
          expect(classNames.indexOf(defaultClassName) >= 0).toEqual(true);
        }
      }
    });
  },

defaultErrorMessages

If the component doesn't correctly handle the classname prop.

If the default classname isn't correctly handled.

List of removed react-northstar tests:

  • ( Basic Tests ) replaced with name-matches-filename, exports-component: * Component exports * Component has a constructor name

  • ( ClassName Tests ) replaced with component-handles-classname:

    • ClassName applies to root element
    • ClassName applies to root component
    • User ClassName does not override the default classes

List of removed ComponentConformance tests:

  • ( Component File Conformance ) replaced with component-handles-classname:

    • Component injects a className prop
  • ( Top Level Component File Conformance ) replaced with exported-top-level, has-top-level-file:

    • Component has a top level file for each file in the directory

Disabled V8 Components

  • Breadcrumb: Doesn't apply the test className to the root.
  • Callout: Doesn't apply the test className to the root.
  • TeachingBubble: Doesn't apply the test className to the root.
  • ContextualMenu: Requires createPortal.
  • Rating: Doesn't apply the defaultClassName to the root.
  • Callout: Doesn't apply the test className to the root.
  • BaseSelectedItemsList: Doesn't apply the test className to the root.
  • SelectedPeopleList: Doesn't apply the test className to the root.
  • ButtonGrid: Doesn't apply the test className to the root.
  • Modal: Requires createPortal.
  • PersonaPresence: Doesn't apply the test className to the root.
  • Coachmark: Doesn't apply the test className to the root.
  • Dialog: Refers to modalProps.className.
  • Nav: Doesn't apply the test className to the root.
  • Panel: Doesn't apply the test className to the root.
  • Sticky: Doesn't apply the test className to the root.
  • SelectionZone: Doesn't apply the test className to the root.
  • CalendarDayGrid: Doesn't apply the test className to the root.

@DustyTheBot
Copy link

DustyTheBot commented Oct 27, 2020

Warnings
⚠️ There are no updates provided to CHANGELOG. Ensure there are no publicly visible changes introduced by this PR.

Generated by 🚫 dangerJS against 03c0e67

@dzearing
Copy link
Member

dzearing commented Feb 1, 2021

Thanks, @ecraig12345 can you please merge once you've signed off?

'component-handles-classname': (componentInfo: ComponentDoc, testInfo: IsConformantOptions) => {
const { Component, wrapperComponent, helperComponents = [], requiredProps, customMount = mount } = testInfo;

const defaultEl = customMount(<Component {...requiredProps} />);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix this but each test (it()) should be standalone, and mounting components should happen inside the test itself.

@ecraig12345 ecraig12345 merged commit 0d0fe56 into microsoft:master Feb 4, 2021
@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/theme-samples@v8.0.0-beta.48 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-monaco-editor@v1.0.0-beta.48 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-experiments@v8.0.0-beta.53 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-docsite-components@v8.0.0-beta.49 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-charting@v5.0.0-beta.49 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-cards@v1.0.0-beta.48 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/azure-themes@v8.0.0-beta.48 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/public-docsite-resources@v1.0.2 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-examples@v1.0.4 has been released which incorporates this pull request.:tada:

Handy links:

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

Successfully merging this pull request may close these issues.

None yet

8 participants