[Doc Enhancement] Introduce MarkdownElement #2224 #2226

frankf-cgn · 2015-11-20T10:22:28Z

First working draft to implement #2224. Result can be seen on <ListsPage>.

In order to test, cd /docs, run npm install and npm run start. Visit http://localhost:3000/#/components/lists and scroll down to "Headline 1"

shaurya947 · 2015-11-23T19:52:26Z

I think this is a slightly better system than what we have today. Obviously, it is easier to write a docs page as a separate markdown file vs. using <p>, <h1> and <Codeblock> elements inside the source code. Moreover, using this approach would be more robust when we think of a multi-lingual docs site: #1673

oliviertassinari · 2015-11-23T20:57:51Z

docs/src/app/components/pages/components/lists.jsx

@@ -15,6 +16,8 @@ const EditorInsertChart = require('svg-icons/editor/insert-chart');
 const FileFolder = require('svg-icons/file/folder');
 const MoreVertIcon = require('svg-icons/navigation/more-vert');
 import { SelectableContainerEnhance } from 'material-ui/hoc/selectable-enhance';
+require('github-markdown-css/github-markdown.css');


I think that this should be move to markdown-element.jsx

your right, there it belongs!

frankf-cgn · 2015-11-24T10:25:48Z

@oliviertassinari Thanks for your feedback!
All the stuff in docs/src/app/components/pages/components/lists.jsx was just there to illustrate the usage, I'm going to remove it.
I think I have to write some doc on how to use the <MarkdownElement>, but for now, I don't no where to put it.
Another open point is the css/style used within the Element. It's github-style and not very "material-design". Some adjustments seem to be appropriate. I'll work on this within the next days.

shaurya947 · 2015-11-24T16:24:57Z

@frankf-cgn I can see people using this component in their applications. So perhaps it makes sense to document it as a new component on its own page? @oliviertassinari what say?

frankf-cgn · 2015-11-24T16:34:27Z

@shaurya947 Strictly taken, this is not a ui-component in "material-design". It's more like <CodeBlock>, <ComponentDoc> or <ComponentInfo>, so I'm not sure if this is a good fit.

shaurya947 · 2015-11-25T17:03:21Z

I'm not sure.. Perhaps you are right, but this could be pretty useful for people making text-heavy web apps.

@oliviertassinari what do you think about making this a documented component with its own page like other components?

@frankf-cgn looking forward to the style changes!

EDIT: just spoke to @hai-cea and seems like it's not adherent to the Material Design spec to treat this like a regular MUI component.

oliviertassinari · 2015-11-26T16:05:13Z

just spoke to @hai-cea and seems like it's not adherent to the Material Design spec to treat this like a regular MUI component.

I agree, let's keep this component for our doc.

frankf-cgn · 2015-11-30T16:38:44Z

@shaurya947 @oliviertassinari After pulling my hair over the lint-rules i finally managed to get this linted.

I have done the following stuff:

reorganize the paths for the markdown files
removed the package github-markdown
added a mui-github-markdown.css based on github-markdown.css with adjusted fonts settings and heading-styles
reworked parts ot the <ListPage> as a showcase on the usage of <MarkdownElement>

Just out of curiosity I played around with various possibilities on how to format the documentation of props in markdown. May ask for some feedback of you both on that?

oliviertassinari · 2015-11-30T18:00:33Z

After pulling my hair over the lint-rules

Sorry, but this made me laught. I have enforced more rules lately 😬. I aim to follow at least eslint:recommended and may be https://github.com/rackt/eslint-config-rackt.

May ask for some feedback of you both on that

I will have a look 👍

frankf-cgn · 2015-11-30T19:54:10Z

docs/src/app/components/markdown-element.jsx

+    const {text} = this.props;
+    const html = marked(text || '');
+
+/* eslint-disable */


To allow html-tags within markdown (e.g. for definition lists) dangerouslySetInnerHTML is needed which is prevented in .eslintL74 no-danger.
I think this is a non-issue here, because we truts the PR of the developer anyway.

oliviertassinari · 2015-12-01T12:55:46Z

docs/package.json

@@ -15,8 +15,10 @@
  },
  "dependencies": {
    "codemirror": "^5.5.0",
+    "hightlight.js": "^8.9.1",


'hightlight.js' is not in the npm registry.

frankf-cgn · 2015-12-01T16:40:02Z

@oliviertassinari The ".js" is correct. but I have had a typo in "hightlight.js", that is now fixed and the style-loader is added, too.

oliviertassinari · 2015-12-03T12:03:24Z

I have this following issue with webpack when running npm start on the doc folder.

ERROR in ./~/highlight.js/lib/languages/autoit.js
Module parse failed: /Users/oliviertassinari/material-ui/docs/node_modules/highlight.js/lib/languages/autoit.js Maximum call stack size exceeded
You may need an appropriate loader to handle this file type.

frankf-cgn · 2015-12-03T12:34:32Z

@oliviertassinari I have no idea, why these npm-modules are installed globally in my environment - have to check this. Sorry for the noise it generates.
Despite I cannot reproduce the error you get with highlight.js/autoit.js I have found several hints how to mitigate this if it occures. I have modified the webpack-config accordingly. Can you check it out if it works for you now?

oliviertassinari · 2015-12-04T22:40:35Z

@frankf-cgn With npm@3 and node@5, it's working 👍.

oliviertassinari · 2015-12-06T11:31:17Z

@frankf-cgn I'm gonna add your MarkdownElement this is a great component!
And use it for #2382.

oliviertassinari · 2015-12-06T12:16:05Z

@frankf-cgn I'm done, could you have a look at the other PR?
I think that we are on the right track 😬

frankf-cgn · 2015-12-06T12:55:53Z

@oliviertassinari Thats awesome! I'll look into the other PR.

frankf-cgn force-pushed the issue/2224 branch from 62ea2e0 to edd7eb5 Compare November 20, 2015 10:23

oliviertassinari added the Review: review-needed label Nov 21, 2015

oliviertassinari reviewed Nov 23, 2015
View reviewed changes

oliviertassinari added PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI and removed Review: review-needed labels Nov 24, 2015

frankf-cgn force-pushed the issue/2224 branch 3 times, most recently from f29aaf2 to f368cae Compare November 25, 2015 08:36

frankf-cgn force-pushed the issue/2224 branch 2 times, most recently from 68ebca1 to 0e1ca0e Compare November 30, 2015 16:30

frankf-cgn reviewed Nov 30, 2015
View reviewed changes

oliviertassinari added Review: review-needed and removed PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI labels Dec 1, 2015

oliviertassinari reviewed Dec 1, 2015
View reviewed changes

frankf-cgn force-pushed the issue/2224 branch from 0e1ca0e to f6620fa Compare December 1, 2015 16:37

[Doc Enhancement] Introduce MarkdownElement mui#2224

a8d45b3

frankf-cgn force-pushed the issue/2224 branch from f6620fa to a8d45b3 Compare December 3, 2015 12:31

oliviertassinari mentioned this pull request Dec 5, 2015

[docs] Autogenerate the property description for the AppBar #2382

Merged

oliviertassinari closed this Dec 6, 2015

alitaheri removed the PR: Needs Review label Feb 15, 2016

zannager added the docs Improvements or additions to the documentation label Mar 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Doc Enhancement] Introduce MarkdownElement #2224 #2226

[Doc Enhancement] Introduce MarkdownElement #2224 #2226

frankf-cgn commented Nov 20, 2015

shaurya947 commented Nov 23, 2015

oliviertassinari Nov 23, 2015

frankf-cgn Nov 24, 2015

frankf-cgn commented Nov 24, 2015

shaurya947 commented Nov 24, 2015

frankf-cgn commented Nov 24, 2015

shaurya947 commented Nov 25, 2015

oliviertassinari commented Nov 26, 2015

frankf-cgn commented Nov 30, 2015

oliviertassinari commented Nov 30, 2015

frankf-cgn Nov 30, 2015

oliviertassinari Dec 1, 2015

frankf-cgn commented Dec 1, 2015

oliviertassinari commented Dec 3, 2015

frankf-cgn commented Dec 3, 2015

oliviertassinari commented Dec 4, 2015

oliviertassinari commented Dec 6, 2015

oliviertassinari commented Dec 6, 2015

frankf-cgn commented Dec 6, 2015

[Doc Enhancement] Introduce MarkdownElement #2224 #2226

[Doc Enhancement] Introduce MarkdownElement #2224 #2226

Conversation

frankf-cgn commented Nov 20, 2015

shaurya947 commented Nov 23, 2015

oliviertassinari Nov 23, 2015

Choose a reason for hiding this comment

frankf-cgn Nov 24, 2015

Choose a reason for hiding this comment

frankf-cgn commented Nov 24, 2015

shaurya947 commented Nov 24, 2015

frankf-cgn commented Nov 24, 2015

shaurya947 commented Nov 25, 2015

oliviertassinari commented Nov 26, 2015

frankf-cgn commented Nov 30, 2015

oliviertassinari commented Nov 30, 2015

frankf-cgn Nov 30, 2015

Choose a reason for hiding this comment

oliviertassinari Dec 1, 2015

Choose a reason for hiding this comment

frankf-cgn commented Dec 1, 2015

oliviertassinari commented Dec 3, 2015

frankf-cgn commented Dec 3, 2015

oliviertassinari commented Dec 4, 2015

oliviertassinari commented Dec 6, 2015

oliviertassinari commented Dec 6, 2015

frankf-cgn commented Dec 6, 2015