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

docs(ComponentDoc): refactor and optimize #2123

Merged
merged 6 commits into from
Oct 20, 2017

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Sep 26, 2017

Splitted from #2070 for the easier review of proposed changes.

This PR is part of work (#1981, #2012) that improves docs usability and performance. Contains the deep refactor of ComponentDoc.

Naming

This PR includes refactor of ComponentDoc. As before, I've refactored it to smaller components and move down showPropsFor to ComponentProps. But I met naming problem and I hope for your help there.

  • ComponentProps => ComponentTable - after performed refactoring, this component has been renamed, since by context it is a table with of props
  • ComponentProps/* => ComponentProp/* - there are numerous small components that represent props, I moved them there and it's what makes me doubt

Alexander Fedyashov added 2 commits September 26, 2017 15:10
…React into docs/refactor-component-doc

# Conflicts:
#	docs/app/Components/ComponentDoc/ComponentProp/ComponentPropEnum.js
parsed.props = _.sortBy(parsed.props, 'name')

result[relativePath] = parsed
result[componentName] = parsed
Copy link
Member Author

@layershifter layershifter Sep 28, 2017

Choose a reason for hiding this comment

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

posixPath is now calculated by plugin. For key in docgenInfo.json will be used a component name, it's unique, so we don't to regexps to find the component info ✌️

Copy link
Member

Choose a reason for hiding this comment

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

We need to make it windows compatible, so it needs to use Noed's path module for path modification. We cannot hardcode forward or backward slashes.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've shipped a small change, looks correct. I don't have a windows PC near and I'm not familar with path. If you have better solution, feel free to push updates.

@layershifter layershifter force-pushed the docs/refactor-component-doc branch from ca97829 to fb6bee0 Compare September 28, 2017 08:32
@codecov-io
Copy link

codecov-io commented Sep 28, 2017

Codecov Report

Merging #2123 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2123   +/-   ##
=======================================
  Coverage   99.76%   99.76%           
=======================================
  Files         150      150           
  Lines        2597     2597           
=======================================
  Hits         2591     2591           
  Misses          6        6
Impacted Files Coverage Δ
src/modules/Search/SearchResult.js 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6153ae7...8af0be6. Read the comment docs.

@layershifter
Copy link
Member Author

@levithomason It's another try, it's different from that you see in #2070.

withDocInfo

Now we have the HOC that generates all needed info about component, later we can easily refactor it to support codesplitting 👍

@levithomason
Copy link
Member

There are still some windows build breaking forward slashes in here, but we'll address those if we hear from windows users.

@levithomason levithomason merged commit e95ee2c into master Oct 20, 2017
@levithomason levithomason deleted the docs/refactor-component-doc branch October 20, 2017 00:40
@levithomason
Copy link
Member

Released in semantic-ui-react@0.76.0

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.

3 participants