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

fix:(ui-patterns): show actual class name passed #663

Merged
merged 4 commits into from
Jul 14, 2023

Conversation

wesleyboar
Copy link
Member

@wesleyboar wesleyboar commented Jun 17, 2022

Overview

In UI Patterns, for Sections, render user actual class names passed, not the compiled classnames used by the app.

Related

None

Changes

  • do not pass compiled classname
  • pass actual classname

Testing

  1. Open UI Patterns.
  2. Find Section patterns.
  3. Confirm the following description lists do not show _some-class_9asdf8, but just some_class:
    • Scrollable
    • With a Complex Table
    • All Properties

UI

Not convenient to provide. Environment broken. I noticed the bug while working on tup-ui's ui-patterns, fixed it there, then ported the fix to here.

@codecov
Copy link

codecov bot commented Jun 17, 2022

Codecov Report

Merging #663 (b93abea) into main (837101a) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #663   +/-   ##
=======================================
  Coverage   64.33%   64.33%           
=======================================
  Files         426      426           
  Lines       12537    12537           
  Branches     2510     2510           
=======================================
  Hits         8066     8066           
  Misses       4255     4255           
  Partials      216      216           
Flag Coverage Δ
javascript 68.70% <ø> (ø)
unittests 60.42% <ø> (ø)

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

Impacted Files Coverage Δ
...UIPatterns/UIPatternsSection/UIPatternsSection.jsx 25.00% <ø> (ø)

wesleyboar added a commit to TACC/tup-ui that referenced this pull request Jun 17, 2022
jchuahtacc added a commit to TACC/tup-ui that referenced this pull request Jun 22, 2022
* 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>
jchuahtacc added a commit to TACC/tup-ui that referenced this pull request Jul 25, 2022
* 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>
@rstijerina rstijerina merged commit 73dc8b7 into main Jul 14, 2023
@rstijerina rstijerina deleted the fix/incorrect-data-for-ui-patterns-user branch July 14, 2023 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants