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

Build docs using vue-styleguidist #439

Merged
merged 2 commits into from
Jun 17, 2019
Merged

Build docs using vue-styleguidist #439

merged 2 commits into from
Jun 17, 2019

Conversation

juliushaertl
Copy link
Contributor

Next try to come up with some documentation. Unfortunately vuepress didn't fit that well, so this replaces #272

I gave different tools a try and ended up with https://vue-styleguidist.github.io/, which generates documentation from code and allows interactive example rendering with live editing capabilities.

For preview I currently setup a build for this branch on netlify: https://nextcloud-vue-components.netlify.com

We could already get this in as a starting point, as it already allows browsing the existing components with props, events.

@juliushaertl juliushaertl added 3. to review Waiting for reviews feature: documentation Related to the documentation labels Jun 16, 2019
@@ -20,6 +20,21 @@
-
-->

<docs>
Copy link
Contributor

Choose a reason for hiding this comment

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

Like it, so simple!!!

@@ -91,6 +110,9 @@ const allowedChildren = [
'ActionText'
]

/**
* @since 0.10.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeeeesss!!

@skjnldsv
Copy link
Contributor

A few travis errors :)

@juliushaertl
Copy link
Contributor Author

A few travis errors :)

Fixed.

I also tried to add some documentation for the Actions component, props of all others as well some very basic examples for Modal and Avatar. I guess we can improve the actual docs step by step once this is merged.

I just encountered that netlify also generates a preview of the documentation for every pull request, that is then linked in the status check list: https://deploy-preview-439--nextcloud-vue-components.netlify.com/

Vue.prototype.t = window.t
Vue.prototype.n = window.n
Vue.prototype.OC = window.OC
Vue.prototype.OCA = window.OCA
Copy link
Contributor

Choose a reason for hiding this comment

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

For what is this needed?

If it's only for lint checks, extending plugin:nextcloud/recommended in eslint configuration should be better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably only t and OC are required, but they don't hurt as long as we don't ship translations with the components:
https://github.com/nextcloud/nextcloud-vue/search?l=Vue&q=t%28%27core%27
https://github.com/nextcloud/nextcloud-vue/search?q=getCurrentUser&unscoped_q=getCurrentUser

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, OC probably also not needed since it is accessing the global object directly without the Vue object as a proxy.

Copy link
Contributor

@korelstar korelstar Jun 17, 2019

Choose a reason for hiding this comment

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

I don't like that app developers have to care about internals from the used library. We should add a mixin like this to nc-vue and use it in those components which use t or similar. App developers then don't have to add this prototype code anymore. They can even use this mixin, too, when they want to use t in their app.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, sounds good. I just added it here as I encountered it in the docs build.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fine, I can do a PR for the mixin, later (in the next days, I think).

@juliushaertl
Copy link
Contributor Author

Codacy/PR Quality Review — Not up to standards. This pull request quality could be better.

@skjnldsv Those seem to be only the minified server styles I've added for now, since otherwise the demo components look quite odd, as we still rely on the shipped CSS. I'd say those can be ignored.

@skjnldsv
Copy link
Contributor

So, merging?

@skjnldsv skjnldsv requested a review from korelstar June 17, 2019 13:28
Copy link
Contributor

@korelstar korelstar left a comment

Choose a reason for hiding this comment

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

🎉

@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jun 17, 2019
@skjnldsv
Copy link
Contributor

All good @juliushaertl?

@juliushaertl
Copy link
Contributor Author

Yep, good to go 🚢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish feature: documentation Related to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants