styleguidist / react-styleguidist

Isolated React component development environment with a living style guide
https://react-styleguidist.js.org/
MIT License
10.84k stars 1.44k forks source link

Para components should use `theme.fontSize.base` or `theme.fontSize.text` #1661

Open juanca opened 4 years ago

juanca commented 4 years ago

Current behavior

When stylegudist is configured to use a font size for base and/or text to be anything other than 16px, the paragraph content generated from markdown does not use the specified font size.

Question: Is a markdown paragraph considered to be either base or text? I am assuming it should be text at the very least.

To reproduce

  1. Config:

    module.exports = {
      theme: {
        fontSize: {
          base: 24,
          text: 36,
        },
      },
    };
  2. Some markdown file:

    # I am an H1
    
    I am just a paragraph! My font size should be really big!

https://github.com/styleguidist/example/pull/8

Expected behavior

The generated paragraph content should use the theme provided.

Or there should be some explanation of how the "theme" is defined: How is a paragraph not "text" or "base"?

juanca commented 4 years ago

At the moment, my solution is to duplicate the theme as a direct style to the Para component. Something like:

styles: {
  Para: { para: {
    fontSize: 14,
  }},
}
sapegin commented 4 years ago

I think it has inherit because it's used in Props where it should be smaller, and I think it shouldn't be used here:

https://github.com/styleguidist/react-styleguidist/blob/53bc4bb357133e2208ca28e5181b44306fdc4e3d/src/client/rsg-components/Props/PropsRenderer.tsx#L24

And the text would be the correct size.

Feel free to send a pull request with a fix! 👾

ghost commented 4 years ago

Hello, I would like to be assigned to this issue.

sapegin commented 4 years ago

@senadglojnaric sure, go for it! 👍

ghost commented 4 years ago

I have a question about testing. I ran a test on my clone repo without changing anything in the code. Like an initial test. And it failed.

Here's my debug log file:

0 info it worked if it ends with ok 1 verbose cli [ 1 verbose cli 'C:\Program Files\nodejs\node.exe', 1 verbose cli 'C:\Program Files\nodejs\node_modules\npm\bin\npm-cli.js', 1 verbose cli 'run', 1 verbose cli 'lint' 1 verbose cli ] 2 info using npm@6.14.8 3 info using node@v12.18.4 4 verbose run-script [ 'prelint', 'lint', 'postlint' ] 5 info lifecycle react-styleguidist@11.0.0-beta2~prelint: react-styleguidist@11.0.0-beta2 6 info lifecycle react-styleguidist@11.0.0-beta2~lint: react-styleguidist@11.0.0-beta2 7 verbose lifecycle react-styleguidist@11.0.0-beta2~lint: unsafe-perm in lifecycle true 8 verbose lifecycle react-styleguidist@11.0.0-beta2~lint: PATH: C:\Program Files\nodejs\node_modules\npm\node_modules\npm-lifecycle\node-gyp-bin;F:\repo\react-styleguidist\node_modules.bin;C:\Program Files\nodejs\node_modules\npm\node_modules\npm-lifecycle\node-gyp-bin;F:\repo\react-styleguidist\node_modules.bin;C:\Program Files (x86)\Common Files\Oracle\Java\javapath;C:\ProgramData\Oracle\Java\javapath;C:\Program Files\Common Files\Microsoft Shared\Windows Live;C:\Program Files (x86)\Common Files\Microsoft Shared\Windows Live;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Program Files (x86)\Windows Live\Shared;C:\Program Files\Microsoft SQL Server\110\Tools\Binn\;C:\Program Files\Microsoft SQL Server\120\Tools\Binn\;C:\Program Files (x86)\Skype\Phone\;C:\ProgramData\chocolatey\bin;C:\Users\GameDev\AppData\Roaming\Python\Python38\Scripts;C:\Program Files\nodejs\;C:\Program Files\Git\cmd;C:\Program Files\MySQL\MySQL Shell 8.0\bin\;C:\Users\GameDev\AppData\Local\Programs\Python\Python38-32\Scripts\;C:\Users\GameDev\AppData\Local\Programs\Python\Python38-32\;C:\Users\GameDev\AppData\Local\Programs\Microsoft VS Code\bin;C:\Users\GameDev\AppData\Local\GitHubDesktop\bin;C:\Users\GameDev\AppData\Roaming\Python\Python38\Scripts;C:\Users\GameDev\AppData\Roaming\npm 9 verbose lifecycle react-styleguidist@11.0.0-beta2~lint: CWD: F:\repo\react-styleguidist 10 silly lifecycle react-styleguidist@11.0.0-beta2~lint: Args: [ '/d /s /c', 'eslint . --cache --fix --ext .js,.ts,.tsx' ] 11 silly lifecycle react-styleguidist@11.0.0-beta2~lint: Returned: code: 1 signal: null 12 info lifecycle react-styleguidist@11.0.0-beta2~lint: Failed to exec lint script 13 verbose stack Error: react-styleguidist@11.0.0-beta2 lint: eslint . --cache --fix --ext .js,.ts,.tsx 13 verbose stack Exit status 1 13 verbose stack at EventEmitter. (C:\Program Files\nodejs\node_modules\npm\node_modules\npm-lifecycle\index.js:332:16) 13 verbose stack at EventEmitter.emit (events.js:315:20) 13 verbose stack at ChildProcess. (C:\Program Files\nodejs\node_modules\npm\node_modules\npm-lifecycle\lib\spawn.js:55:14) 13 verbose stack at ChildProcess.emit (events.js:315:20) 13 verbose stack at maybeClose (internal/child_process.js:1021:16) 13 verbose stack at Process.ChildProcess._handle.onexit (internal/child_process.js:286:5) 14 verbose pkgid react-styleguidist@11.0.0-beta2 15 verbose cwd F:\repo\react-styleguidist 16 verbose Windows_NT 6.1.7601 17 verbose argv "C:\Program Files\nodejs\node.exe" "C:\Program Files\nodejs\node_modules\npm\bin\npm-cli.js" "run" "lint" 18 verbose node v12.18.4 19 verbose npm v6.14.8 20 error code ELIFECYCLE 21 error errno 1 22 error react-styleguidist@11.0.0-beta2 lint: eslint . --cache --fix --ext .js,.ts,.tsx 22 error Exit status 1 23 error Failed at the react-styleguidist@11.0.0-beta2 lint script. 23 error This is probably not a problem with npm. There is likely additional logging output above. 24 verbose exit [ 1, true ]

I'm asking this because I found a solution for this issue #1661 and I wanted to run testing and send pull request. Then I ran into this testing problem. I also tried to clone repo again and start a test without changes. Same thing. Could you give me a hand with this?

apennell commented 4 years ago

@senadglojnaric It looks like what's happening is there's a lint issue. If you run npm run lint, does that show you specific lint errors instead of just erroring out like above?

ghost commented 4 years ago

Hello, first of all, thanks for helping me! I ran npm run lint without any errors. Then after that, I ran npm test and at the end the test ended with errors suggested that I run npm run test:jest -- -u.

After that I get this message to the console:

Test Suites: 6 failed, 114 passed, 120 total Tests: 22 failed, 599 passed, 621 total Snapshots: 205 passed, 205 total Time: 41.027s Ran all test suites. npm ERR! code ELIFECYCLE npm ERR! errno 1 npm ERR! react-styleguidist@11.0.0-beta2 test:jest: jest npm ERR! Exit status 1 npm ERR! npm ERR! Failed at the react-styleguidist@11.0.0-beta2 test:jest script. npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in: npm ERR! C:\Users\GameDev\AppData\Roaming\npm-cache_logs\2020-10-02T20_36_26_234Z-debug.log npm ERR! Test failed. See above for more details.

It seems that 6 Testsuits and 22 Tests are failing. Snapshots are all OK.

This is how my debug.log looks now:

0 info it worked if it ends with ok 1 verbose cli [ 1 verbose cli 'C:\Program Files\nodejs\node.exe', 1 verbose cli 'C:\Program Files\nodejs\node_modules\npm\bin\npm-cli.js', 1 verbose cli 'run', 1 verbose cli 'test:jest' 1 verbose cli ] 2 info using npm@6.14.8 3 info using node@v12.18.4 4 verbose run-script [ 'pretest:jest', 'test:jest', 'posttest:jest' ] 5 info lifecycle react-styleguidist@11.0.0-beta2~pretest:jest: react-styleguidist@11.0.0-beta2 6 info lifecycle react-styleguidist@11.0.0-beta2~test:jest: react-styleguidist@11.0.0-beta2 7 verbose lifecycle react-styleguidist@11.0.0-beta2~test:jest: unsafe-perm in lifecycle true 8 verbose lifecycle react-styleguidist@11.0.0-beta2~test:jest: PATH: C:\Program Files\nodejs\node_modules\npm\node_modules\npm-lifecycle\node-gyp-bin;F:\repo\react-styleguidist\node_modules.bin;C:\Program Files\nodejs\node_modules\npm\node_modules\npm-lifecycle\node-gyp-bin;F:\repo\react-styleguidist\node_modules.bin;C:\Program Files (x86)\Common Files\Oracle\Java\javapath;C:\ProgramData\Oracle\Java\javapath;C:\Program Files\Common Files\Microsoft Shared\Windows Live;C:\Program Files (x86)\Common Files\Microsoft Shared\Windows Live;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Program Files (x86)\Windows Live\Shared;C:\Program Files\Microsoft SQL Server\110\Tools\Binn\;C:\Program Files\Microsoft SQL Server\120\Tools\Binn\;C:\Program Files (x86)\Skype\Phone\;C:\ProgramData\chocolatey\bin;C:\Users\GameDev\AppData\Roaming\Python\Python38\Scripts;C:\Program Files\nodejs\;C:\Program Files\Git\cmd;C:\Program Files\MySQL\MySQL Shell 8.0\bin\;C:\Users\GameDev\AppData\Local\Programs\Python\Python38-32\Scripts\;C:\Users\GameDev\AppData\Local\Programs\Python\Python38-32\;C:\Users\GameDev\AppData\Local\Programs\Microsoft VS Code\bin;C:\Users\GameDev\AppData\Local\GitHubDesktop\bin;C:\Users\GameDev\AppData\Roaming\Python\Python38\Scripts;C:\Users\GameDev\AppData\Roaming\npm 9 verbose lifecycle react-styleguidist@11.0.0-beta2~test:jest: CWD: F:\repo\react-styleguidist 10 silly lifecycle react-styleguidist@11.0.0-beta2~test:jest: Args: [ '/d /s /c', 'jest' ] 11 silly lifecycle react-styleguidist@11.0.0-beta2~test:jest: Returned: code: 1 signal: null 12 info lifecycle react-styleguidist@11.0.0-beta2~test:jest: Failed to exec test:jest script 13 verbose stack Error: react-styleguidist@11.0.0-beta2 test:jest: jest 13 verbose stack Exit status 1 13 verbose stack at EventEmitter. (C:\Program Files\nodejs\node_modules\npm\node_modules\npm-lifecycle\index.js:332:16) 13 verbose stack at EventEmitter.emit (events.js:315:20) 13 verbose stack at ChildProcess. (C:\Program Files\nodejs\node_modules\npm\node_modules\npm-lifecycle\lib\spawn.js:55:14) 13 verbose stack at ChildProcess.emit (events.js:315:20) 13 verbose stack at maybeClose (internal/child_process.js:1021:16) 13 verbose stack at Process.ChildProcess._handle.onexit (internal/child_process.js:286:5) 14 verbose pkgid react-styleguidist@11.0.0-beta2 15 verbose cwd F:\repo\react-styleguidist 16 verbose Windows_NT 6.1.7601 17 verbose argv "C:\Program Files\nodejs\node.exe" "C:\Program Files\nodejs\node_modules\npm\bin\npm-cli.js" "run" "test:jest" 18 verbose node v12.18.4 19 verbose npm v6.14.8 20 error code ELIFECYCLE 21 error errno 1 22 error react-styleguidist@11.0.0-beta2 test:jest: jest 22 error Exit status 1 23 error Failed at the react-styleguidist@11.0.0-beta2 test:jest script. 23 error This is probably not a problem with npm. There is likely additional logging output above. 24 verbose exit [ 1, true ]

And this is an example of one of the failing tests printed in the output (first one):

Summary of all failing tests FAIL src/loaders/utils/tests/getComponentFiles.spec.ts (7.142s) ● getComponentFiles() should accept components as a function that returns file names

expect(received).toEqual(expected) // deep equality

- Expected  - 2
+ Received  + 2

  Array [
-   "~/components/Annotation/Annotation.js",
-   "~/components/Button/Button.js",
+   "~\\components\\Annotation\\Annotation.js",
+   "~\\components\\Button\\Button.js",
  ]

  18 | it('getComponentFiles() should accept components as a function that returns file names', () => {
  19 |      const result = getComponentFiles(() => components, configDir);
> 20 |      expect(deabs(result)).toEqual(processedComponents);
     |                            ^
  21 | });
  22 | 
  23 | it('getComponentFiles() should accept components as a function that returns absolute paths', () => {

  at Object.toEqual (src/loaders/utils/__tests__/getComponentFiles.spec.ts:20:24)
ghost commented 4 years ago

I went through all the test output that had failed. It seems that they all have one thing in common. They check filename paths. I'm using Windows and paths in tests use Unix convention. I guess, there is a difference between how path separators are formatted ( \ or / ).

apennell commented 4 years ago

Oh good find! Are you able to resolve your issue then?

ghost commented 4 years ago

I think so. I'm going to ignore failing tests for now. Windows and Linux both read forwardslash and backslash in filepaths. But in the tests, they are compared as strings.

Anyway, I'm going to send pull request ignoring these tests.