docs(custom-components): adding initial guide for creating custom components by mnajdova · Pull Request #517 · microsoft/fluent-ui-react

mnajdova · 2018-11-22T15:10:10Z

This PR adds guide for how the custom components can be created using the renderComponent functionality in Stardust.
To do

update the changelog

codecov · 2018-11-22T15:18:16Z

Codecov Report

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

@@           Coverage Diff           @@
##           master     #517   +/-   ##
=======================================
  Coverage   88.17%   88.17%           
=======================================
  Files          42       42           
  Lines        1455     1455           
  Branches      212      187   -25     
=======================================
  Hits         1283     1283           
  Misses        167      167           
  Partials        5        5

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 53da689...688a80d. Read the comment docs.

bmdalex · 2018-11-22T16:03:21Z

docs/src/views/CustomComponents.tsx

+      ].join('\n')}
+    />
+
+    <p>Let's go step by step throughout all bits from the renderComponent method.</p>


~~renderComponent~~ -> createComponent
I also changed this in the CHANGELOG.md file that seems to mention renderComponent as well
https://github.com/stardust-ui/react/pull/518/files#r235773446

layershifter · 2018-11-23T09:58:42Z

docs/src/views/CustomComponents.tsx

+    const componentProps = _.pick(props, 'children')
+    return <button className={cx(props.className, classes.root)} {...componentProps} />
+  },
+})


I think that we should omit the usage of lodash in such basic examples:

const StyledButton: React.SFC<StyledButtonProps> = createComponent<StyledButtonProps, {}>({ displayName: 'StyledButton', render({ className, stardust, ...props }) { const { classes } = stardust return <button {...rest} className={cx(className, classes.root)} /> }, })

As I see, my code does the same thing or no?

We are applying in this case the styles and variables to the button, but I get your point, will change it not to use lodash.

Something like this?

const StyledButton: React.SFC<StyledButtonProps> = createComponent<StyledButtonProps, {}>({ displayName: 'StyledButton', render({ stardust, className, children }) { const { classes } = stardust const componentProps = { children } return <button className={cx(className, classes.root)} {...componentProps} /> }, })

-return <button className={cx(className, classes.root)} {...componentProps} /> +return <button className={cx(className, classes.root)}>{children}</button>

👍

kuzhelov · 2018-11-23T15:41:39Z

docs/src/views/CustomComponents.tsx

+})
+
+export default () => (
+  <DocPage title="Custom Components">


maybe something like "Integrate Custom Components" - because currently this is not absolutely clear from header whether we are suggesting to develop custom components or integrate existing ones

kuzhelov · 2018-11-23T15:43:01Z

docs/src/views/CustomComponents.tsx

+  <DocPage title="Custom Components">
+    <Header as="h2" content="Overview" />
+    <p>
+      You can use your own components as part of the Stardust's theming mechanism. In order for all


'styling and theming'

kuzhelov · 2018-11-23T15:45:38Z

docs/src/views/CustomComponents.tsx

+      value={[
+        `import { createComponent } from '@stardust-ui/react'`,
+        ``,
+        `export interface StyledButtonProps {`,


I would suggest to remove TS from this example - main reason is that it introduces quite a lot of 'noise', and additional one (and the one that applies to broader community) is that our docs should be readable for folks that have no prior knowledge of TS

kuzhelov · 2018-11-23T15:49:33Z

docs/src/views/CustomComponents.tsx

+    </p>
+
+    <p>
+      The first argument to the <code>createComponent</code> config's param is the displayName. This


would suggest to also wrap displayName and Provider in <code/>

kuzhelov · 2018-11-23T15:53:12Z

docs/src/views/CustomComponents.tsx

+
+    <p>
+      The first argument to the <code>createComponent</code> config's param is the displayName. This
+      name is very important, because that's the key that will be used in the Provider's theme if


we could simplify this sentence a bit:

is the displayName, which value might be used as key to define component's styles and variables in theme, exactly the same way how it might be done for any first-class Stardust component.

I think that this will provide enough thrill to the reader, so one will fully understand the importance of this prop ;)

kuzhelov · 2018-11-23T15:57:56Z

docs/src/views/CustomComponents.tsx

+        `<Provider`,
+        `  theme={{`,
+        `    componentStyles: {`,
+        `      StyledButton: {`,


would suggest to swap variables and styles sections, as the later one uses data defined in former.

kuzhelov · 2018-11-23T15:58:25Z

docs/src/views/CustomComponents.tsx

+    />
+
+    <p>
+      The second argument of the <code>createComponent</code> config param is the render method.


lets wrap render in <code> as well

kuzhelov · 2018-11-23T16:00:35Z

docs/src/views/CustomComponents.tsx

+
+    <p>
+      The second argument of the <code>createComponent</code> config param is the render method.
+      This is the place where you link the Stardust bits with your custom component. This method is


nit-nit:

... where you might link Stardust bits with your custom component - e.g. by simply passing them as props. This render method will be invoked with the following parameters:

kuzhelov · 2018-11-23T16:01:02Z

docs/src/views/CustomComponents.tsx

+    </p>
+    <ul>
+      <li>
+        <code>stardust</code> - this is the object containing the evaluated theming props (classes


nit: 'this is' is not necessary, can be safely omitted.

classes and rtl should be wrapped in <code />

kuzhelov · 2018-11-23T16:02:52Z

docs/src/views/CustomComponents.tsx

+    </p>
+    <ul>
+      <li>
+        <code>stardust</code> - this is the object containing the evaluated theming props (classes


lets end each item's text with dot

kuzhelov · 2018-11-23T16:04:37Z

docs/src/views/CustomComponents.tsx

+      </li>
+    </ul>
+    <p>
+      The previous implementation of the render method inside the <code>createComponent</code>{' '}


sorry, not sure that can completely understand what this paragraph is about - why previous and, also, it seems that there are too many commas there

This paragraph is not really necessary as we already covered all important bits in the paragraph before the list items :)

kuzhelov · 2018-11-23T16:05:39Z

docs/src/views/CustomComponents.tsx

+
+    <p>
+      We already saw how the <code>Provider</code> can define some stylings and variables for the
+      custom components. Next, we will take a look into few examples of how the user can further


'few' -> 'several/couple' (as 'few' has rather 'not enough' semantics :)

kuzhelov · 2018-11-23T16:06:49Z

docs/src/views/CustomComponents.tsx

+    <p>
+      We already saw how the <code>Provider</code> can define some stylings and variables for the
+      custom components. Next, we will take a look into few examples of how the user can further
+      customize the styling and variables of the custom components, the same way they would do with


nit: maybe simplified:
customize the styling and variables -> customize styles and variables of these components

kuzhelov · 2018-11-23T16:09:55Z

docs/src/views/CustomComponents.tsx

+      the Stardust components.
+    </p>
+
+    <Header as="h3" content="Example 1. Using the variables property" />


would omit the and use <code>:

<Header as="h3" content={<>Example 1. Using <code>variables</code> property</>} />

also, would rather expect to see section devoted to styles prop first, as this is a first thing customer would learn about Stardust - with variables being rather more advanced concept

kuzhelov · 2018-11-23T16:10:46Z

docs/src/views/CustomComponents.tsx

+      render={() => (
+        <Provider
+          theme={{
+            componentStyles: {


same here - would suggest to swap places of componentVariables and componentStyles sections

kuzhelov · 2018-11-23T16:12:53Z

docs/src/views/CustomComponents.tsx

+      )}
+    />
+
+    <Header as="h3" content="Example 2. Using the styles property" />


just 'using styles property' should be enough

kuzhelov · 2018-11-23T16:17:11Z

docs/src/views/CustomComponents.tsx

+
+    <ExampleSnippet
+      value={[
+        `<StyledButton styles={({theme: {siteVariables}}) => ({backgroundColor: siteVariables.black})}>`,


I would rather strive for simplifying examples as much as possible. We should just show that it is possible to use styles prop, as well as suggest the fact that it could be used the same way as for Stardust components. Then I see us providing simple example that might prove that, and after this example make a sentence that all other advanced scenarios are supported as well, please, take a look on the corresponding docs.

This will allow us

to simplify example's code (crucially important thing)

not introduce additional maintainability burden to us - say, in case if implementation of styles prop change, we won't need to fix this example's code

kuzhelov · 2018-11-23T16:21:38Z

docs/src/views/CustomComponents.tsx

+
+    <ExampleSnippet
+      value={[
+        `<StyledButton variables={(siteVars) => ({color: siteVars.black})}>`,


lets try to simplify it a bit - while I do understand that this example is aimed to cover all things that are possible to do with this prop being introduced, at the same time, while being in the client's shoes, I would rather expect something pretty simple that would just deliver necessary idea to me. Otherwise, just imagine that I am not yet familiar with siteVariables and what the beast it is - should I learn that or, maybe, it is fine for me to use variables without this knowledge? I would say that the later is just fine

mnajdova · 2018-11-23T17:29:52Z

@kuzhelov thanks much for the review! All comments resolved :)

layershifter · 2018-11-26T09:05:08Z

docs/src/views/IntegrateCustomComponents.tsx

+import * as React from 'react'
+import * as cx from 'classnames'
+import { NavLink } from 'react-router-dom'
+import { Header } from 'semantic-ui-react'


I think that we can use own Header component there 😃

Switching the theme to dark will make the Header component white, and we don't have the other themes implemented for all page (like making the background dark in dark-mode). I would not like to introduce unnecessary providers just for handling things like that. In all docs pages, the Headers are from semantic-ui, so when the dark mode will be implemented for all page, we can safely replace them all in one iteration.

layershifter · 2018-11-26T09:05:40Z

docs/src/views/IntegrateCustomComponents.tsx

+  className?: string
+  styles?: ComponentSlotStyle
+  variables?: ComponentVariablesInput
+  children?: React.ReactNodeArray | React.ReactNode


We have own type for this, ReactChildren

layershifter · 2018-11-26T09:06:27Z

docs/src/views/IntegrateCustomComponents.tsx

+  children?: React.ReactNodeArray | React.ReactNode
+}
+
+const StyledButton: React.SFC<StyledButtonProps> = createComponent<StyledButtonProps, {}>({


The state is optional.

Suggested change

const StyledButton: React.SFC<StyledButtonProps> = createComponent<StyledButtonProps, {}>({

const StyledButton: React.SFC<StyledButtonProps> = createComponent<StyledButtonProps>({

layershifter · 2018-11-26T09:08:13Z

docs/src/views/IntegrateCustomComponents.tsx

+        `      StyledButton: {`,
+        `        root: ({ props, variables, theme: {siteVariables} }) => ({`,
+        `          backgroundColor: siteVariables.brand,`,
+        `          color: (variables as any).color,`,


Suggested change

` color: (variables as any).color,`,

` color: variables.color,`,

Can we omit any this in example's code?

layershifter · 2018-11-26T09:09:31Z

docs/src/views/IntegrateCustomComponents.tsx

+              StyledButton: {
+                root: ({ props, variables, theme: { siteVariables } }) => ({
+                  backgroundColor: siteVariables.brand,
+                  color: (variables as any).color,


The same thing, ^

We cannot omit it here, but it is removed from the string displayed as a code in the docs page.

kuzhelov · 2018-11-26T11:49:58Z

docs/src/views/IntegrateCustomComponents.tsx

+    />
+
+    <p>
+      Let's go step by step throughout all bits from the <code>createComponent</code> method.


nit: '.. from the createComponent ..' => '... of the createComponent ...'

kuzhelov · 2018-11-26T11:51:36Z

docs/src/views/IntegrateCustomComponents.tsx

+        and <code>rtl</code>).
+      </li>
+      <li>
+        <code>...props</code> - all other props provided by the user.


nit: maybe user props would be enough

kuzhelov · 2018-11-26T11:54:00Z

docs/src/views/IntegrateCustomComponents.tsx

+
+    <ExampleSnippet
+      value={[
+        `<StyledButton variables={{color: "black" }}>`,


just to be sure - could you, please, suggest where is the code that suggests to the user where this variable is used (i.e. styles code)?

Added paragraph with Provider and example componentStyles that would be applied.

kuzhelov · 2018-11-26T11:59:32Z

docs/src/views/IntegrateCustomComponents.tsx

+    />
+
+    <p>
+      All other advanced scenarios for applying styles and variables are supported as well. For more


Would suggest to write it this way:

for each example (Styles and Variables) introduce the concluding paragraph that would tell the following (essentially, the though expressed in this paragraph: 'Note that all other scenarios for applying styles/variables are supported as well - please, take a look on the corresponding ... section for more details.')

introduce additional example where theming support will be mentioned (primitive example is necessary):

const customTheme = { // component's displayName arg is used as a selector StyledButton: { ... } } <Provider theme={customTheme}> <StyledButton> </Provider>

and, the same way, conclude with a similar paragraph saying 'for more advanced theming scenarios, please, take a look on the corresponding Theming section'

…amples

# Conflicts: # CHANGELOG.md

-added custom components guide in the docs

61828a4

mnajdova requested review from dvdzkwsk, kuzhelov, levithomason and miroslavstastny as code owners November 22, 2018 15:10

-updated changelog

6874613

bmdalex reviewed Nov 22, 2018

View reviewed changes

bmdalex approved these changes Nov 22, 2018

View reviewed changes

manajdov added 2 commits November 22, 2018 18:16

-replaced renderComponent with createComponent

7e0d276

-replaced renderComponent with createComponent

c7647d4

layershifter reviewed Nov 23, 2018

View reviewed changes

-simplified StyledButton example code

02702da

kuzhelov reviewed Nov 23, 2018

View reviewed changes

-addressed comments from PR

b06fc4f

layershifter reviewed Nov 26, 2018

View reviewed changes

-addressed PR comments

08af6fd

kuzhelov reviewed Nov 26, 2018

View reviewed changes

kuzhelov approved these changes Nov 26, 2018

View reviewed changes

manajdov added 5 commits November 26, 2018 14:48

-addressed final PR comments

788baf8

-updated changelog

603e424

-adding descriptive comments for the displayName in the Provider's ex…

5f01ec8

…amples

Merge branch 'master' into docs/custom-components

738e616

# Conflicts: # CHANGELOG.md

-small lexical change

688a80d

mnajdova merged commit d33711e into master Nov 26, 2018

mnajdova mentioned this pull request Nov 27, 2018

docs(Prototype): Chat messages with reactions popover prototype #524

Merged

layershifter deleted the docs/custom-components branch January 10, 2019 11:26

	const StyledButton: React.SFC<StyledButtonProps> = createComponent<StyledButtonProps, {}>({
	const StyledButton: React.SFC<StyledButtonProps> = createComponent<StyledButtonProps>({

	` color: (variables as any).color,`,
	` color: variables.color,`,

Conversation

mnajdova commented Nov 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Nov 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

layershifter Nov 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mnajdova Nov 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kuzhelov Nov 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kuzhelov Nov 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kuzhelov Nov 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mnajdova commented Nov 23, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mnajdova commented Nov 22, 2018 •

edited

Loading

codecov bot commented Nov 22, 2018 •

edited

Loading

layershifter Nov 23, 2018 •

edited

Loading

mnajdova Nov 23, 2018 •

edited

Loading

kuzhelov Nov 23, 2018 •

edited

Loading

kuzhelov Nov 23, 2018 •

edited

Loading

kuzhelov Nov 23, 2018 •

edited

Loading

kuzhelov Nov 26, 2018 •

edited

Loading