Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

docs: improve docs for shorthand props #751

Merged
merged 12 commits into from
Jan 22, 2019
Merged

Conversation

kuzhelov
Copy link
Contributor

@kuzhelov kuzhelov commented Jan 21, 2019

TODO

  • update changelog


<p>
So, what this string value (i.e. {code("'chess rook'")}) means for {code('Button')}{' '}
component and, essentially, how it transforms into rendered {code('<Icon />')} element?
Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion, this question doesn't bring any value to the user. Let's try to remove it and maybe provide bullet points for explaining this points below.

<p>
There is even shorter way to define default element's props - by using a primitive value.
In that case provided shorthand value will be taken as a value for 'default prop' of this
default element.
Copy link
Contributor

Choose a reason for hiding this comment

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

'default prop' of this default element => 'default prop' of this element. Would avoid the default for the element as well...

])}
<p>
This works because {code('name')} is the default prop of slot's {code('<Icon />')}{' '}
element.
Copy link
Contributor

Choose a reason for hiding this comment

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

This works because {code('name')} is the default prop of the {code('')} component.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

a bit reluctant to use this terminology, as default props for Component may lead to false associations. Essentially, default element's prop is even more correct, given how this default prop's value is handled

{code('accessibility')}).
</strong>{' '}
This is because, in contrast to other forms of shorthand values, Stardust-evaluated props
cannot be unsafely passed to the element which type is, generally, doesn't allow Stardust
Copy link
Contributor

Choose a reason for hiding this comment

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

cannot be safely?


<p>Thus, in its simplest form, it could be used the following way:</p>

{codeExample([`<Button icon={() => <Icon name='chess rook' />} />`])}
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure that this example brings any value... Let's try not to add examples which should never be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this one provides a visibility of what does it mean to satisfy criterias of this function. If we will start right with the render callback stuff, it won't be absolutely clear what is going on - there might be a false assumption from the client that this 'render' function is, actually, triggers rendering somewhere, while, in fact, it just returns an element.

I would really like to leave this simple explanation of what does it mean to pass function as a shorthand value, and what Stardust expects from this function.

What do you think?

` /* how to render */`,
` (ComponentType, props) => (`,
` <div class='my-icon-container'>`,
` <i {...props} />`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can showcase here the usage of the styles and accessibility, like spreading the accessbility.root elements on the i element or using the classes generated?

const subheader = content => <Header as="h3">{content}</Header>

const code = content => <code>{content}</code>
const codeExample = snippet => <CodeSnippet value={joinToString(snippet)} />
Copy link
Member

Choose a reason for hiding this comment

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

I have a strange feeling about there helpers I understand why there are there, but I also should mention that they are not looking friendly for newcomers 🤔

@kuzhelov kuzhelov merged commit 2166d21 into master Jan 22, 2019
@kuzhelov kuzhelov deleted the docs/improve-shorthand-docs branch January 22, 2019 13:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants