FP-1647: Use Core-Styles #639
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #639 +/- ##
==========================================
+ Coverage 67.71% 67.72% +0.01%
==========================================
Files 421 421
Lines 12910 12914 +4
Branches 2375 2378 +3
==========================================
+ Hits 8742 8746 +4
Misses 3882 3882
Partials 286 286
Flags with carried forward coverage won't be shown. Click here to find out more.
|
just a few more, not all of them; poc, this; i can do more if requested
This was referenced May 15, 2022
|
oops was looking at the wrong branch, disregard my previous message |
|
i almost ignored your message... I re-installed packages, just to be sure (negligible change) |
rstijerina
reviewed
May 25, 2022
wesleyboar
commented
May 25, 2022
There was a problem hiding this comment.
This is working, but I've expressed concern over using both source files (by global CSS) and dist files (by module CSS), but J.R. seemed okay with it, and J.Y. did not have feeling strong enough to comment.
Just thoughts. Anyway, I also noticed something to fix before I merge: dependency on transient branch.1
Footnotes
This was referenced Jun 1, 2022
wesleyboar
added a commit
to TACC/tup-ui
that referenced
this pull request
Jun 1, 2022
CEPv2 barely uses SASS. It's only use is mixins and nesting.
There are ways to do both features that are on the web standards path.
Both features can be done with POstCSS
- mixins can be replaced by PostCSS `@extend`:
TACC/Core-Portal#639
- there is a nesting plugin (I need to test it more)
* docs(fp-1647): npm link to live edit core-styles * docs: improve npm link instructions Source: TACC/Core-CMS#4249f5b * docs(README): reduce npm link steps * docs(README): remove CMS-specific instruction * docs: simplify npm link instructions and command * docs(README): polish npm link steps - add commit to core-styles step - link to npm install caveat explanation
wesleyboar
commented
Jun 2, 2022
| @@ -10,6 +10,8 @@ Override Bootstrap styles. See: | |||
|
|
|||
| Styleguide Components.Bootstrap.Modal | |||
| */ | |||
| @import url('@tacc/core-styles/source/_imports/tools/x-truncate.css'); | |||
There was a problem hiding this comment.
This path will change slightly if we use tup-ui (#651).
| @@ -2,9 +2,9 @@ | |||
| /* SEE: ./README.md */ | |||
|
|
|||
| /* SETTINGS */ | |||
| @import url('@tacc/core-styles/source/_imports/settings/border.css'); | |||
There was a problem hiding this comment.
This path will change slightly if we use tup-ui (#651).
| @@ -19,8 +19,8 @@ | |||
|
|
|||
| /* OBJECTS */ | |||
| /* NOTE: For the most part, allow React components to explicitely import as needed */ | |||
| @import url('@tacc/core-styles/source/_imports/objects/o-flex-item-table-wrap.css'); | |||
There was a problem hiding this comment.
This path will change slightly if we use tup-ui (#651).
| --(…)light: lighter value | ||
| --(…)dark: darker value | ||
| */ | ||
| @import url('@tacc/core-styles/source/_imports/settings/color.css'); |
There was a problem hiding this comment.
This path will change slightly if we use tup-ui (#651).
|
|
||
| /* Usage: `var(--global-font-family--sans)` */ | ||
| /* SEE: https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_custom_properties */ | ||
| @import url('@tacc/core-styles/source/_imports/settings/font.css'); |
There was a problem hiding this comment.
This path will change slightly if we use tup-ui (#651).
|
|
||
| /* Usage: `var(--global-space--section-right)` */ | ||
| /* SEE: https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_custom_properties */ | ||
| @import url('@tacc/core-styles/source/_imports/settings/space.css'); |
There was a problem hiding this comment.
This path will change slightly if we use tup-ui (#651).
| @@ -0,0 +1,4 @@ | |||
| @import url('@tacc/core-styles/source/_imports/tools/media-queries.css'); | |||
There was a problem hiding this comment.
This path will change slightly if we use tup-ui (#651).
jarosenb
approved these changes
Jun 2, 2022
jchuahtacc
pushed a commit
to TACC/tup-ui
that referenced
this pull request
Jun 8, 2022
* fix: clarify commands in README From: jarosenb/core-nx-demo#1 * feat(wip): integrate TACC/Core-Styles Status: no build output for lib/core-styles * fix(wip): integrate TACC/Core-Styles Status: build gives libs/core-styles/dist but no /dist/libs/core-styles * fix: accurate, consistent core-styles/package.json * fix: remove excess nx cwd option from core-styles * Revert "fix: clarify commands in README" This reverts commit fa9472a. * fix(wip): core-styles + TypeScript (no conversion) TypeScript is supported again without converting existing .js files. Status: dist output appears usable (can npm ci & npm build inside it) Problem: libs/core-styles/dist is not output to root dist * docs: core-styles README and TODO headings * fix: include & exclude correct core-styles assets Status: dist output appears usable (can npm install & build inside it) * docs: update outdated comment * feat: remove SASS (core-components, generators) CEPv2 barely uses SASS. It's only use is mixins and nesting. There are ways to do both features that are on the web standards path. Both features can be done with POstCSS - mixins can be replaced by PostCSS `@extend`: TACC/Core-Portal#639 - there is a nesting plugin (I need to test it more) * chore(version): v0.6.0-alpha * docs(changelog): update TACC/Core-Styles links * fix: README install commands * chore(version): v0.6.0-alpha, package(-lock).json * feat(core-styles): root build includes local build * chore(core-styles): version bump dist css * feat(core-styles): publish process & remove dist/ * chore(core-styles): version bump 0.6.0-alpha.1 * fix(core-styles): add missing file to package path typo caused by neglect to update path upon file move * chore(core-styles): version bump 0.6.0-alpha.2 * refactor(core-styles): move dependencies to root change comments of core-styles dependencies * fix(core-styles): remove outdated command * chore(core-styles): formate:write (limited) I can not accept all the things Prettier is doing to me CSS. I must configure before I let Prettier take control. * chore(core-styles): formate:write (html + one md) Let prettier control HTML files. Also let in one README.md's changes. * feat(core-styles): formate:write to ignore css via .prettierignore * feat(core-styles): formate:write on REAME.md's * chore(core-styles): let npm order dependencies * feat(core-styles): stylelint, config it recommends * chore(core-styles): format:write * fix(core-styles): restore deps root→core-styles CMS breaks if this is done. CMS relies on Styles dependencies. This is a known deficiency that I want to resolve. * fix(core-styles): no format css if core-styles cwd Ignore CSS when formatter is run within core-styles directory. Without this css is only ignored if formatter is run from root. * fix(core-styles): remove recommended lint rules I will add these back when I need to automate linting. Right now its just me, and this integration needs to be complete. * fix(core-styles): remove recommended lint rules I will add these back when I need to automate linting. Right now its just me, and this integration needs to be complete. * fix(core-styles): restore npm ci (cuz deps moved) * feat(core-styles): update button from Core-Portal - TACC/Core-Portal#631 - TACC/Core-Portal#654 * fix(core-styles): format write * docs(README): module usage: fix typo, drop old opt - Fix a mistake in require command. - Drop outdated `fileExt` option. Source: TACC/Core-Styles@660ce17
wesleyboar
added a commit
to TACC/tup-ui
that referenced
this pull request
Jun 8, 2022
jchuahtacc
added a commit
to TACC/tup-ui
that referenced
this pull request
Jun 16, 2022
* 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>
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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview:
Install Core-Styles and use it in various places.
Requires:
Required by:
Related Jira tickets:
Summary of Changes:
Testing Steps & UI Photos:
Verify
.modal-titletruncates text with ellipsis.Open a modal, make the title very long (edit code or live markup), and verify title is truncated with an ellipsis.
Verify Dashboard and Manage Account still have responsive design.
Adjust with the screen width and height. Some reused media query values now load from Core-Styles.
Verify UI colors and borders are unchanged.
If this failed, colors would be wrong. The borders and grayscale colors now load from Core-Styles.
Verify tables show available content and do not overlap.
If this failed, tables would not show content. The
o-flex-item-table-wrapfix now loads from Core-Styles.Verify
Expand's.headertruncates text with ellipsis.Open a job's View Details modal, edit "System Output" heading text to be very long (edit code or live markup), and verify it is truncated with an ellipsis.
Verify
ShowMoretruncation and reveal works.Open "UI Patterns" section, scroll to "Show More", and verify the text is truncated at 4 lines with an ellipsis.
Click the "Show More" button link, and verify the text is not truncated.
Verify
DescriptionListis Truncated.Open "UI Patterns" section, scroll to "DescriptionList", and verify the text is truncated for:
Notes:
I limited code changes for testing Core-Styles. I can do more if you want.
I intentionally don't use Core-Styles everywhere in this PR, so PR is digestible. If you want me to add more relevant changes to this PR—e.g. replace all static media queries with custom ones, use extend for even more truncation—just let me know. 🙂
The diff is very large because of
package-lock.json. Seems unavoidable.I installed three new dependencies. The
@tacc/core-styles(Core-Styles) dependency is required (or else there is no PR). Each dependency shares a similar large dependency tree, and adds ~8k lines topackage-lock.json, but together they add less than any one adds individually.