FP-1650: Test Cases for _common/Button #640

wesleyboar · 2022-05-17T00:04:25Z

Overview:

Add test cases for button since refining it during FP-1491 styling.

Related Jira tickets:

FP-1650

Summary of Changes:

export and use type map and size map
test cases for Button complexities

Testing Steps:

Verify UI is unchanged — see FP-1491: Style _common Button React Component #598
Verify test cases pass.

UI Photos:

Top	Bottom	Misc. States

codecov · 2022-05-17T00:06:09Z

Codecov Report

Merging #640 (e3219c3) into main (6b6e4c1) will increase coverage by 2.72%.
The diff coverage is 100.00%.

❗ Current head e3219c3 differs from pull request most recent head 7b4e41f. Consider uploading reports for the commit 7b4e41f to get more accurate results

@@            Coverage Diff             @@
##             main     #640      +/-   ##
==========================================
+ Coverage   67.79%   70.51%   +2.72%     
==========================================
  Files         426      216     -210     
  Lines       13173     5809    -7364     
  Branches     2414     1625     -789     
==========================================
- Hits         8930     4096    -4834     
+ Misses       3951     1635    -2316     
+ Partials      292       78     -214

Flag	Coverage Δ
javascript	`70.51% <100.00%> (+0.13%)`	⬆️
unittests	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
client/src/components/_common/Message/Message.jsx	`97.22% <ø> (ø)`
client/src/components/_common/Button/Button.jsx	`83.33% <100.00%> (+18.02%)`	⬆️
client/src/components/_common/Icon/Icon.jsx	`100.00% <100.00%> (ø)`
server/portal/apps/webhooks/urls.py
server/portal/apps/projects/managers/base.py
server/portal/apps/notifications/urls.py
server/portal/apps/workspace/api/views.py
server/portal/apps/search/tasks.py
server/portal/libs/elasticsearch/indexes.py
...r/portal/apps/workspace/migrations/0001_initial.py
... and 204 more

wesleyboar

notes for reviewers

wesleyboar · 2022-05-17T00:05:32Z

client/src/components/_common/Button/Button.test.js

-  it('does not render button without text', () => {
-    const { queryByTestId } = render(
-      <Button data-testid="no button here"></Button>
+  it('uses given text', () => {
+    muteTypeNotLinkNoSizeLog();
+    const { getByTestId } = render(
+      <Button>{TEST_TEXT}</Button>
    );
-    const el = queryByTestId('no button here');
-    expect(el).toBeNull;
+    expect(getByTestId('text').textContent).toEqual(TEST_TEXT);
  });
-  it('disables button when in loading state', () => {
-    const { queryByText } = render(
-      <Button isLoading={true}>Loading Button</Button>
-    );
-    const el = queryByText('Loading Button');
-    expect(el).toBeDisabled;


Moved: Loading tests are near the bottom of the file.

wesleyboar · 2022-05-17T00:06:50Z

client/src/components/_common/Button/Button.jsx

-  const buttonRootClass = styles['root'];
-
-  let buttonTypeClass;
-  if (type === 'link') {
-    buttonTypeClass = styles['as-link'];
-  } else if (type === 'primary' || type === 'secondary') {
-    buttonTypeClass = styles[`${type}`];
-  } else if (type === '') {
-    buttonTypeClass = type;
-  }
-
-  let buttonSizeClass;
-  if (size === 'small') {
-    buttonSizeClass = styles['size-small'];
-  } else {
-    buttonSizeClass = styles[`width-${size}`];
-  }
-
-  const buttonLoadingClass = isLoading ? styles['loading'] : '';
-


DRY-er: Now that type and size maps exist, this can be simplified to logic directly in className.

wesleyboar · 2022-05-17T00:07:40Z

client/src/components/_common/Button/Button.jsx

@@ -24,6 +35,7 @@ const Button = ({
  iconNameAfter,
  type,
  size,
+  dataTestid,


Extra: So that other elements can identify their <Button> instance in test cases.

wesleyboar · 2022-05-17T00:09:05Z

client/src/components/_common/Button/Button.test.js

+      if (isPropertyLimitation(type, TEST_SIZE)) {
+        return Promise.resolve();
+      }


Explanation: Postpone testing complex cases until later, in 'property limitation'.

wesleyboar · 2022-05-17T00:10:19Z

client/src/components/_common/Icon/Icon.jsx

@@ -2,27 +2,35 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import './Icon.module.css';

-const Icon = ({ children, className, name }) => {
+const Icon = ({ children, className, dataTestid, name }) => {


Explanation: So <Button /> tests can distinguish between the two icons it can (simultaneously) have.

wesleyboar · 2022-05-17T16:00:23Z

I tried to follow GitHub's command-line instructions for managing a merge conflict. It gave me errors and my attempt to solve the errors gave the PR 68 commits and changes to 10 files. Um, no. I'll take it from here, GitHub… I locally re-created this branch using cherry-pick, then force-pushed to this remote branch. 2 commits, 3 files changed.

edmondsgarrett

Looks like there is a merge conflict here.

Button does not need to have a class. This msitake came form me in #631.

wesleyboar

I have noted that where tests that appear deleted is actually just test being moved.

client/src/components/_common/Button/Button.test.js

wesleyboar · 2022-06-02T23:16:24Z

Thanks, Garret. Resolved.

Source: TACC/Core-Portal#640

edmondsgarrett

Looks good!

client/src/components/_common/Button/Button.jsx

Also changed instances in two other components.

wesleyboar · 2022-06-13T23:19:49Z

I don't understand the code coverage fail. I added test cases, and one of the failed runs shows I increased coverage. 🤷‍♂️

…:TACC/Core-Portal into task/FP-1650-button-component-test-cases

Source: https://github.com/TACC/tup-ui/pull/7/files#diff-7c7d6d82c4485c00f402e951eb76d77bfad3a2f3b567fa9f2330eb47a85f814a

This reverts commit e80f9de.

* feat(core-components): copy & polish from cepv2 These comp…s: - were copied from cepv2 - were made more modular - have react-based dependencies Not cepv2 comp…s were copied, because some are not easy to make modular. Comp…s were copied May 4, 2022. Some have since changed in CEPv2, e.g.: - Button - Paginator - (maybe) Message * feat(comp…s): cepv2 update but w/ new styles paths Source: TACC/Core-Portal#639 * feat(comp…s): 2nd cepv2 update (no path updates) * feat(comp…s): cepv2 update: button tests, unmerged Source: TACC/Core-Portal#640 * fix(comp…s): fix core-styles paths (libs → lib) * feat(comp…s): update all paths to core-styles src Some of these may be able to use dist... I haven't checked yet. * feat(comp…s): fix paths that can use styles dist One of the paths is still src/lib/_imports. CMS has this problem often. To use src/lib/_imports instead of dist, see TUP-274. * fix(styles): all src/lib imports to use rel. paths Avoid users needing resolution paths specific to core-styles hierarchy. * fix(styles): src/lib rel. paths need no _imports/ Hotfix previous commit: aab21ba "fix(styles): all src/lib imports to use rel. paths" * fix(tup-ui): load CSS from correct path * Convert Button and dependencies to TS * Add reactstrap global * Replace temporary Message component to exports * Use Button from core-components in tup-ui * Added babel-plugin-postcss for core-components * Formatting * linting * Formatting * Task/tup 272 core components vite (#8) * Vite library build for core-components * Icon allows react node children * Add testing-library/react * Fix tests * Fix test * Formatting * vite output directory * clean tup-ui on build Co-authored-by: Wesley Bomar <wbomar@tacc.utexas.edu>

* task/TUP-272 - core components fixup (#7) * feat(core-components): copy & polish from cepv2 These comp…s: - were copied from cepv2 - were made more modular - have react-based dependencies Not cepv2 comp…s were copied, because some are not easy to make modular. Comp…s were copied May 4, 2022. Some have since changed in CEPv2, e.g.: - Button - Paginator - (maybe) Message * feat(comp…s): cepv2 update but w/ new styles paths Source: TACC/Core-Portal#639 * feat(comp…s): 2nd cepv2 update (no path updates) * feat(comp…s): cepv2 update: button tests, unmerged Source: TACC/Core-Portal#640 * fix(comp…s): fix core-styles paths (libs → lib) * feat(comp…s): update all paths to core-styles src Some of these may be able to use dist... I haven't checked yet. * feat(comp…s): fix paths that can use styles dist One of the paths is still src/lib/_imports. CMS has this problem often. To use src/lib/_imports instead of dist, see TUP-274. * fix(styles): all src/lib imports to use rel. paths Avoid users needing resolution paths specific to core-styles hierarchy. * fix(styles): src/lib rel. paths need no _imports/ Hotfix previous commit: aab21ba "fix(styles): all src/lib imports to use rel. paths" * fix(tup-ui): load CSS from correct path * Convert Button and dependencies to TS * Add reactstrap global * Replace temporary Message component to exports * Use Button from core-components in tup-ui * Added babel-plugin-postcss for core-components * Formatting * linting * Formatting * Task/tup 272 core components vite (#8) * Vite library build for core-components * Icon allows react node children * Add testing-library/react * Fix tests * Fix test * Formatting * vite output directory * clean tup-ui on build Co-authored-by: Wesley Bomar <wbomar@tacc.utexas.edu> * task/TUP-280 -- UI patterns (#12) * Add UI-Patterns app * Section exports from core-components * DescriptionList * Messages * Paginator * Pill * Dropdown selector * Same tsconfig settings in tup-ui * Show more * Section and Infinite Scroll Table * Add components for Sidebar (disabled) * Add react-router-dom v6 * Sidebar * Formatting and linting * linting for core-components * task/TUP-280, 282, 283 -- UI patterns (fixes), CSS vars, styles (#14) * Add UI-Patterns app * Section exports from core-components * DescriptionList * Messages * Paginator * Pill * Dropdown selector * Same tsconfig settings in tup-ui * Show more * Section and Infinite Scroll Table * Add components for Sidebar (disabled) * Add react-router-dom v6 * Sidebar * Formatting and linting * linting for core-components * fix(core-components): import failures 1. Load from `src/lib/_imports/`: - Can't load core-styles from its `dist`. - I don't know why. - I do know `.gitignore` is not the problem.\* \* I tested disabling it's `dist` entry. 2. Add required CSS file from Portal: - Portal used `components/bootstrap.form.css`. - CMS did not, but CMS started Core Styles. - So Core Styles did not have `…/bootstrap.form.css`. * fix(core-styles): dist ignore comment, README typo 1. Fix the comment about dist in `.gitignore`. 2. Fix the path inaccuracy in `README`. * fix(core-components): css syntax & missing values * feat: postcss config & deps Tested only with: - `nx build core-components` - `nx serve ui-patterns` * fix(core-components): do not use scss * docs(core-styles): css lint & syntax highlight * fix(core-styles): missing css vars from portal * fix(core-components): explicitely import css vars * fix(core-components): no css var within calc() Such a variable cannot be reduced: https://github.com/postcss/postcss-calc#usage Without reduction, i.e. if var stays, var definition must be preserved: https://github.com/csstools/postcss-plugins/tree/main/plugins/postcss-custom-properties#preserve If var preserved, then we may be unable to avoid duplicate vars: vitejs/vite#4448 (comment) * fix(ui-patterns): add missing button patterns Patterns were recently added to portal: tertiary, active, loading. They had a couple active state bugs, which I also fixed: - wrong sample code in UI Pattern for Active button - class typo in Button.module.css * fix(core-styles): explicit var import for patterns This should be done to all core-styles stylesheets. But that will have an uncertain effect on CMS. So for now, just make this work for core-components. * fix:(ui-patterns): show actual class name passed Mimicked by: TACC/Core-Portal#663 * fix(core-styles): add missing font family vars * fix(ui-patterns): explicit var import for patterns Just like I did earlier for core-components: 4c0bf2b. * chore(ui-patterns): do not import unused styles * chore(tup-ui): do not import unused styles * docs(ui-patterns): link to ITCSS organization doc * docs(tup-ui): link to ITCSS organization doc * feat(core-styles): add o-fixed-header-table * feat(core-components): use o-fixed-header-table Use o-fixed-header-table for InfiniteScrollTable via core-styles. * feat(core-styles): cortal icons * fix(core-components): icon styles, font, props * fix(ui-patterns): missing space between buttons * feat(core-styles): components/bootstrap.modal.css * feat: install bootstrap ^4.6.0 * fix(ui-patterns): global css (inc. bootstrap) also, re-document (simpler, broader) index.css * fix(core-components): hide Spinner Loading... text * fix(core-components): do not use Reactstrap Button * fix(core-components): (wip) tsx button prop limits Restrict combinations of button props type and size. Works only in file. Does not work in practice: - Use <Button> with bad props in Button.tsx, VS Code complains. - Use <Button> with bad props in UIPatternsButton.tsx, VS Code ignores. Also, removed related test cases, cuz TypeScript prevents need, right? * fix(core-styles): auto width for size-less buttons Set a default width for buttons that: - have no width - have no size - are not links This resolves ac5dcf8 having removed default size. * fix(core-components): mostly no use native button - Do not use native button for typical buttons. - The close button for Messages is atypical. * fix(core-components): ShowMore Button type This was not completely converted from Reactstrap to Core Component. * fix(ui-patterns): nx format:write * fix(core-components): nx format:write * fix(core-styles): nx format:write * fix: match reactstrap version to bootstrap version * Revert "fix(core-components): hide Spinner Loading... text" This reverts commit d5bfc79. Since commit 4a873cb," Loading..." text is automatically hidden. - Reactstrap 9 and Bootstrap 5 use ".visually-hidden" class. - Rectstrap 8 and Bootstrap 4 uses ".sr-only" class. To avoid other unexpected bugs, I suggest same Bootstrap as CEPv2. Or… we reveal and fix any bugs (reference Bootstrap 4 → 5 migration). * fix(core-styles): vertically align button content Why `c-button` not `cortal.icon`? - This must be applied to the text and icon elements to work. Why not use inline-flex et cetera? - Because sibling buttons vertical alignment broken when I tried it. Inspiration: TACC/Core-Portal@307c54a * fix(tup-ui): style links, no use wb-link Style hyperlinks. Remove unused "wb-link" classes. * fix(core-components): message no override .wb-link 1. Message need not overwrite ".wb-link" (class dropped in 08ad3da). 2. Add an active state. \* \* Design does not care to distinguish link states. * fix(ui-patterns): activeClassN… react-r…-dom ver. Use same react-router-dom version as CEPv2 to make activeClassName work. - downgrade react-router-dom - use switch and component props * fix(core-components): Sidebar styles 1. Remove unused class "nav-content". 2. Use anchor tag pseduo classes to overwrite "elements.css". 3. Add missing style for nav content. Depends on: e859114 (i.e. previous commit) * feat(core-components): simpler Sidebar styles 1. No "content" wrapper div. 2. Move "content" wrapper div styles to link. 3. Move text padding to icon. - Because the padding exists only because icon exists. - Required adding a Sidebar "icon" class. Builds off: 2243276 (i.e. previous commit) * chore(ui-patterns): nx format:write * chore(core-comp…s): load form css at dist not src * chore(core-comp…s): load css settings from dist Co-authored-by: Joon-Yee Chuah <jchuah@tacc.utexas.edu> * fix(core-components): missing dependency, dependency alternative (#13) * fix(core-components): CSS and Dependency imports * chore(tup-ui): minimize #13 diff * chore(core-components): minimize diff ie remove space * task/TUP - 284 -- core wrappers (#15) * Port tapis-ui/_wrappers * UIPatternsComplexWizard * Fix components to switch rather than route * Field array of field arrays step * Simple wizard * Formatting and linting * Fix wizard continue * Fix complex wizards value loading * Clarity on initialvalues vs defaultvalues in simple wizard * Formatting * Fix unit tests * fix collapse icon * Replace ErrorMessage component with formtext * Fix config files * Formatting * Add validation to wizards * Bump react-router-dom to v6.3.0 * Version bump reactstrap * navlink active classname Co-authored-by: Wesley Bomar <wbomar@tacc.utexas.edu> Co-authored-by: Wesley B <62723358+wesleyboar@users.noreply.github.com>

client/src/components/_common/Button/Button.jsx

wesleyboar requested a review from rstijerina May 17, 2022 00:04

wesleyboar commented May 17, 2022

View reviewed changes

wesleyboar mentioned this pull request May 17, 2022

FP-1491: Style _common Button React Component #598

Merged

Base automatically changed from task/FP-1491-style-button-component to main May 17, 2022 15:33

wesleyboar marked this pull request as draft May 17, 2022 15:54

wesleyboar added 2 commits May 17, 2022 10:56

test(fp-1650): add test cases, add prop value maps

d5bad95

chore(fp-1650): prettier fix

7499aca

wesleyboar force-pushed the task/FP-1650-button-component-test-cases branch from 6e3466e to 7499aca Compare May 17, 2022 15:57

wesleyboar marked this pull request as ready for review May 17, 2022 16:00

wesleyboar requested a review from edmondsgarrett May 23, 2022 20:13

edmondsgarrett requested changes Jun 2, 2022

View reviewed changes

wesleyboar added 2 commits June 2, 2022 18:12

Merge branch 'main' into task/FP-1650-button-component-test-cases

07a339f

fix(fp-1650): don't fail on empty className

38af56b

Button does not need to have a class. This msitake came form me in #631.

wesleyboar commented Jun 2, 2022

View reviewed changes

client/src/components/_common/Button/Button.test.js Show resolved Hide resolved

client/src/components/_common/Button/Button.test.js Show resolved Hide resolved

wesleyboar requested a review from edmondsgarrett June 2, 2022 23:16

Merge branch 'main' into task/FP-1650-button-component-test-cases

ea23a26

wesleyboar added a commit to TACC/tup-ui that referenced this pull request Jun 8, 2022

feat(comp…s): cepv2 update: button tests, unmerged

af596ef

Source: TACC/Core-Portal#640

edmondsgarrett approved these changes Jun 10, 2022

View reviewed changes

rstijerina reviewed Jun 13, 2022

View reviewed changes

client/src/components/_common/Button/Button.jsx Outdated Show resolved Hide resolved

wesleyboar and others added 2 commits June 13, 2022 18:14

fix(fp-1650): dataTestid default, '' → undefined

a6c81cd

Also changed instances in two other components.

Merge branch 'main' into task/FP-1650-button-component-test-cases

f2c53ea

wesleyboar requested a review from rstijerina June 13, 2022 23:20

wesleyboar added 4 commits June 13, 2022 18:23

fix(fp-1650): remove unused function

17049b0

Merge branch 'task/FP-1650-button-component-test-cases' of github.com…

852b590

…:TACC/Core-Portal into task/FP-1650-button-component-test-cases

fix(fp-1650): fixes from tup-ui PR

e80f9de

Source: https://github.com/TACC/tup-ui/pull/7/files#diff-7c7d6d82c4485c00f402e951eb76d77bfad3a2f3b567fa9f2330eb47a85f814a

Revert "fix(fp-1650): fixes from tup-ui PR"

e24a043

This reverts commit e80f9de.

Merge branch 'main' into task/FP-1650-button-component-test-cases

7b4e41f

rstijerina approved these changes Jun 14, 2022

View reviewed changes

rstijerina merged commit 28aa67d into main Jun 14, 2022

rstijerina deleted the task/FP-1650-button-component-test-cases branch June 14, 2022 16:50

edmondsgarrett reviewed Aug 3, 2022

View reviewed changes

client/src/components/_common/Button/Button.jsx Show resolved Hide resolved

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FP-1650: Test Cases for _common/Button #640

FP-1650: Test Cases for _common/Button #640

wesleyboar commented May 17, 2022

codecov bot commented May 17, 2022 •

edited

Loading

wesleyboar left a comment

wesleyboar May 17, 2022

wesleyboar May 17, 2022

wesleyboar May 17, 2022 •

edited

Loading

wesleyboar May 17, 2022

wesleyboar May 17, 2022

wesleyboar commented May 17, 2022 •

edited

Loading

edmondsgarrett left a comment

wesleyboar left a comment •

edited

Loading

wesleyboar commented Jun 2, 2022

edmondsgarrett left a comment

wesleyboar commented Jun 13, 2022

FP-1650: Test Cases for _common/Button #640

FP-1650: Test Cases for _common/Button #640

Conversation

wesleyboar commented May 17, 2022

Overview:

Related Jira tickets:

Summary of Changes:

Testing Steps:

UI Photos:

codecov bot commented May 17, 2022 • edited Loading

Codecov Report

wesleyboar left a comment

Choose a reason for hiding this comment

wesleyboar May 17, 2022

Choose a reason for hiding this comment

wesleyboar May 17, 2022

Choose a reason for hiding this comment

wesleyboar May 17, 2022 • edited Loading

Choose a reason for hiding this comment

wesleyboar May 17, 2022

Choose a reason for hiding this comment

wesleyboar May 17, 2022

Choose a reason for hiding this comment

wesleyboar commented May 17, 2022 • edited Loading

edmondsgarrett left a comment

Choose a reason for hiding this comment

wesleyboar left a comment • edited Loading

Choose a reason for hiding this comment

wesleyboar commented Jun 2, 2022

edmondsgarrett left a comment

Choose a reason for hiding this comment

wesleyboar commented Jun 13, 2022

codecov bot commented May 17, 2022 •

edited

Loading

wesleyboar May 17, 2022 •

edited

Loading

wesleyboar commented May 17, 2022 •

edited

Loading

wesleyboar left a comment •

edited

Loading