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

Example component #18

Merged
merged 5 commits into from
May 15, 2018
Merged

Example component #18

merged 5 commits into from
May 15, 2018

Conversation

meinstein
Copy link
Contributor

This is to get the ball rolling on dynamically importing examples. That part was straight forward enough. But I have a couple question going fwd:

  1. What is the contents field for each example object intended to contain? Is that for the contents of the source code? If so, it appears to not be working as intended quite yet.
  2. Piggybacking of the previous question - where and how should we display the contents for the rendered example?
  3. Where and how should we gather the description for example?
  4. Lastly, should we just deduce the title for an example based on the file name that contains the example?

@meinstein meinstein requested a review from AlecAivazis May 3, 2018 00:39
@coveralls
Copy link

coveralls commented May 3, 2018

Coverage Status

Coverage remained the same at 69.684% when pulling 7079ddd on examples into bc2b900 on master.

@AlecAivazis
Copy link
Owner

Hey @meinstein -

  1. The contents are supposed to contain the content of the example file. Currently it just shows the body of the default export, although we might want to do something different in order to show the necessary import statements for the example.
  2. The sketch file that we've been working on has an example of what the different examples look like.
  3. I wasn't originally intending for there to be an example apart from the title, if we want to add one it would be very easy to duplicate the logic I included for parsing the title comment out for a description comment aswell.
  4. The title should be included in the example payload, as shown by this test

@meinstein
Copy link
Contributor Author

meinstein commented May 3, 2018

Sup @AlecAivazis -

  1. Ok, after looking at how you did it, I realized what was going on. There's specific ways to export the examples in order for the contents field to work. We should document this somewhere so peeps don't make my mistake. The way I did it originally was:
const Foo = () => <Foo />
export default Foo

and that produced bizarre value for contents. However, doing:

export default () => <Foo />

produced following contents:

() => <Foo />

which is awesome. Having the quark package import statement is def a nice to have, but can also be obviated in the in the getting started section for now. When rendering the contents, I wonder if it's worth stripping the () =>. I'm thinking yes..

  1. Yep, looked at Sketch file again, and jogged my recollection. Thanks.

  2. Looking at how title works with the magic comment as well as revisiting the sketch file, I don't currently foresee a need for the description in addition to the title. Good to know it's easy to implement should it become needed.

  3. This may be also require some documentation, as it seems to be directly related to Question 1. It appears the magic comment is not detected unless the default export is done in a particular way.

@meinstein
Copy link
Contributor Author

meinstein commented May 3, 2018

Another thing - doing the dynamic imports of the examples elicits a bevy of warnings (one for each readme):

WARNING in ./docs/examples/overlay/Overlay/README.md
Module parse failed: Unexpected token (1:9)
You may need an appropriate loader to handle this file type.
| Overlays are temporary containers that span the entire viewport, covering the current content. The are best used for small interactions that happen in the current context.
|
 @ ./docs/examples sync ^\.\/.*$
 @ ./docs/src/views/ComponentDetails/Examples/Example/index.js
 @ ./docs/src/views/ComponentDetails/Examples/index.js
 @ ./docs/src/views/ComponentDetails/index.js
 @ ./docs/src/views/index.js
 @ ./docs/src/index.js
 @ multi (webpack)-dev-server/client?http://localhost:8080 webpack/hot/dev-server ./docs/src/index.js

The dynamic require is going directly to the default export in the respective example file - it never touches the README.md as far as I can tell?

https://github.com/AlecAivazis/quark/pull/18/files#diff-89805ad225eb42127fdeb1c4e198af42R11

@AlecAivazis
Copy link
Owner

AlecAivazis commented May 3, 2018

Yep, the examples are extremely convention driven, It should be documented somewhere for sure. If I get some time this week, i'll throw together a CONTRIBUTING guide so we can outline what all is necessary when building a component.

@AlecAivazis
Copy link
Owner

AlecAivazis commented May 3, 2018

The example files should never include the README. Are you seeing something different? I wouldn't expect that error you have showing since we shouldn't be requiring a markdown file.

@AlecAivazis
Copy link
Owner

Let's use this to get around the README's: https://github.com/cherrry/ignore-loader

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.

3 participants