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

Update docs related to styles #62

Merged
merged 9 commits into from
Sep 11, 2019

Conversation

magicmatatjahu
Copy link
Member

Description

Changes proposed in this pull request:

  • Update docs related to styles:
    • remove doc described old way of styling component and create new one.
  • Update root Readme.md with information about supported version of spec and update with info about styling.

@magicmatatjahu magicmatatjahu added enhancement New feature or request area/documentation Related to all activities around documentation area/community Related to all activities that are done for community labels Sep 10, 2019
README.md Outdated
@@ -2,6 +2,8 @@

[![All Contributors](https://img.shields.io/badge/all_contributors-5-orange.svg?style=flat-square)](#contributors)

> :warning: This package doesn't support AsyncAPI 1.x anymore. We recommend to upgrade to the latest AsyncAPI version using the [AsyncAPI converter](https://github.com/asyncapi/converter). If you need to convert documents on the fly, you may use the [Node.js](https://github.com/asyncapi/converter) or [Go](https://github.com/asyncapi/converter-go) converters.
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe add that npm package up to some version supports v1? and after that only v2

Copy link
Contributor

Choose a reason for hiding this comment

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

we can make something like a v1 branch with old code before migration

Copy link
Member Author

Choose a reason for hiding this comment

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

@aerfio We can create tag for appropriate revision, so this will be easy.

@@ -16,6 +16,7 @@
"changelog": "lerna-changelog",
"clean": "lerna clean",
"start": "lerna exec --parallel -- npm run start",
"test": "cd library && npm test",
Copy link
Contributor

Choose a reason for hiding this comment

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

and maybe && cd .. to go back

Copy link
Member Author

Choose a reason for hiding this comment

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

Command concatenation has no effect on the host.

Copy link
Contributor

Choose a reason for hiding this comment

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

cool, didn't know that :D ignore this comment then

README.md Outdated

---

> :warning: This package doesn't support AsyncAPI 1.x anymore. We recommend to upgrade to the latest AsyncAPI version using the [AsyncAPI converter](https://github.com/asyncapi/converter). If you need to convert documents on the fly, you may use the [Node.js](https://github.com/asyncapi/converter) or [Go](https://github.com/asyncapi/converter-go) converters.
Copy link
Member

Choose a reason for hiding this comment

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

imho this is not a big deal therefore separate warning exposed like that doesn't make much sense to me.
after overview we should just have a section called like supported versions and explain that this component supports 2.0, that people should convert and that is it,
mentioning old version of the component doesn't make much sense as we will not maintain it anyway

};

const App = () => <AsyncApiComponent schema={schema} config={config} />;

render(<App />, document.getElementById("root"));
```

### TypeScript
### JavaScript
Copy link
Contributor

Choose a reason for hiding this comment

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

I know we love TS, but pure JS example should be first, as it was before 😢

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope :P


## Changing styles

In the AsyncApi component we use pure css styling. Each component inside root component has a unique css class. Each class has form: `asyncapi__{ELEMENT}--{MODIFIER}`, where:
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe each html tag has unique css class? from css perspective there are html tags/elements, not components


In the AsyncApi component we use pure css styling. Each component inside root component has a unique css class. Each class has form: `asyncapi__{ELEMENT}--{MODIFIER}`, where:

- `{ELEMENT}` - is a name of specific element. Each element name is the concatenation of the names of the elements in which it is located. For example: `asyncapi__channel-header-title` is located in `header` HTML element of `channel` element.
Copy link
Contributor

Choose a reason for hiding this comment

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

is located in `header` HTML element - is this always true? This may lead to situation in which someone does something like header.asyncapi__channel-header-title {}, but in reality there could be div element. Maybe instead of outlining things like that just say we follow BEM and link to it?

Copy link
Member Author

Choose a reason for hiding this comment

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

But here I described that title is in header, not *-title is HTML header.


For changing styles, you must create (or if you use [default](../../library/src/styles/fiori.css) style, modify) appropriate class.

> **NOTE**: We recommend copy default styles from [here](../../library/src/styles/fiori.css) and changing them at its sole discretion.
Copy link
Contributor

Choose a reason for hiding this comment

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

copying

and did you really mean discretion?

@@ -39,9 +39,8 @@ If you make any changes in the project structure, remember to update it.

Use the following tools to develop the AsyncApi React component:
Copy link
Contributor

@aerfio aerfio Sep 11, 2019

Choose a reason for hiding this comment

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

I've never liked this section, because it's not like we somehow use react or TS cli to develop directly, they are dependencies and are downloaded automatically, I'm not bringing them myself, it's misleading

to be discussed with @kazydek and @magicmatatjahu

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not insisting on keeping it if you think it is more of a hassle than help 🤷‍♀

export const ONE_OF_FOLLOWING_MESSAGES_SUBSCRIBE_TEXT =
'You can send one of the following messages:';
'You can receive one of the following messages:';
Copy link
Member

Choose a reason for hiding this comment

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

I would say subscribe to instead of receive

README.md Outdated

[![All Contributors](https://img.shields.io/badge/all_contributors-5-orange.svg?style=flat-square)](#contributors)

## Overview

A [React](https://reactjs.org/) component for AsyncAPI specification. It allows you to render the documentation of your asynchronous API provided in the AsyncAPI specification format and validate this specification. You can fully restyle the component using your own styles.

> :warning: This package doesn't support AsyncAPI 1.x anymore. We recommend to upgrade to the latest AsyncAPI version using the [Node.js](https://github.com/asyncapi/converter) or [Go](https://github.com/asyncapi/converter-go) converters.
Copy link
Contributor

@kazydek kazydek Sep 11, 2019

Choose a reason for hiding this comment

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

Suggested change
> :warning: This package doesn't support AsyncAPI 1.x anymore. We recommend to upgrade to the latest AsyncAPI version using the [Node.js](https://github.com/asyncapi/converter) or [Go](https://github.com/asyncapi/converter-go) converters.
>**CAUTION:** This package doesn't support AsyncAPI 1.x anymore. We recommend that you upgrade to the latest AsyncAPI version using the [Node.js](https://github.com/asyncapi/converter) or [Go](https://github.com/asyncapi/converter-go) converters.

Copy link
Contributor

Choose a reason for hiding this comment

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

or recommend upgrading

README.md Outdated

> **NOTE:** Use React version 16.0.0 or higher and styled-components version 4.0.0 or higher.
- [`react`](https://github.com/facebook/react/): >= 16.8.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- [`react`](https://github.com/facebook/react/): >= 16.8.0
- [`react`](https://github.com/facebook/react/) (version 16.8.0 or higher)

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this is how we write it in other docs

docs/README.md Outdated
@@ -5,7 +5,7 @@
This directory contains the following documents that relate to the project:

- configuration:
- [Theme Modification](./configuration/theme-modification.md) describes how to edit a theme of the whole AsyncAPI component or of its specific part.
- [Style Modification](./configuration/style-modification.md) describes how to edit the styles of the AsyncApi component.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- [Style Modification](./configuration/style-modification.md) describes how to edit the styles of the AsyncApi component.
- [Style Modification](./configuration/style-modification.md) describes how to edit the styles of the AsyncAPI component.

Copy link
Contributor

Choose a reason for hiding this comment

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

here and everywhere I guess, I noticed now that it is written in lowercase in some places

This field is set to `false` by default.

> **NOTE:** When you set this flag to `true`, you must provide definitions of all styles.
This field contains configuration responsible for initial expanding specific parts of the AsyncApi component.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This field contains configuration responsible for initial expanding specific parts of the AsyncApi component.
This field contains configuration responsible for expanding specific parts of the AsyncApi component automatically.

> **NOTE:** When you set this flag to `true`, you must provide definitions of all styles.
This field contains configuration responsible for initial expanding specific parts of the AsyncApi component.
`root` means root component for specific parts of the AsyncApi component. `Elements` means elements inside `root` component.
By default `expand.channels.root` is set to `true`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
By default `expand.channels.root` is set to `true`.
By default, `expand.channels.root` is set to `true`.


In the AsyncApi component we use pure css styling. Each component inside root component has a unique css class. Each class has form: `asyncapi__{ELEMENT}--{MODIFIER}`, where:

- `{ELEMENT}` - is a name of specific element. Each element name is the concatenation of the names of the elements in which it is located. For example: `asyncapi__channel-header-title` is located in `header` HTML element of `channel` element.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- `{ELEMENT}` - is a name of specific element. Each element name is the concatenation of the names of the elements in which it is located. For example: `asyncapi__channel-header-title` is located in `header` HTML element of `channel` element.
- `{ELEMENT}` is the name of a specific element. Each element name is the concatenation of the names of the elements in which it is located. For example, `asyncapi__channel-header-title` is located in the `header` HTML element of the `channel` element.

In the AsyncApi component we use pure css styling. Each component inside root component has a unique css class. Each class has form: `asyncapi__{ELEMENT}--{MODIFIER}`, where:

- `{ELEMENT}` - is a name of specific element. Each element name is the concatenation of the names of the elements in which it is located. For example: `asyncapi__channel-header-title` is located in `header` HTML element of `channel` element.
- `{MODIFIER}` - is a modifier for `{ELEMENT}`. Very few elements have a modifier. This is usually badge, button and similar, generic components.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- `{MODIFIER}` - is a modifier for `{ELEMENT}`. Very few elements have a modifier. This is usually badge, button and similar, generic components.
- `{MODIFIER}` is a modifier for `{ELEMENT}`. Very few elements have a modifier. This is usually a badge, button, or similar, generic components.

- `{ELEMENT}` - is a name of specific element. Each element name is the concatenation of the names of the elements in which it is located. For example: `asyncapi__channel-header-title` is located in `header` HTML element of `channel` element.
- `{MODIFIER}` - is a modifier for `{ELEMENT}`. Very few elements have a modifier. This is usually badge, button and similar, generic components.

For changing styles, you must create (or if you use [default](../../library/src/styles/fiori.css) style, modify) appropriate class.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For changing styles, you must create (or if you use [default](../../library/src/styles/fiori.css) style, modify) appropriate class.
To change styles, create an appropriate class or modify it if you use the [default](../../library/src/styles/fiori.css) style.


For changing styles, you must create (or if you use [default](../../library/src/styles/fiori.css) style, modify) appropriate class.

> **NOTE**: We recommend copy default styles from [here](../../library/src/styles/fiori.css) and changing them at its sole discretion.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
> **NOTE**: We recommend copy default styles from [here](../../library/src/styles/fiori.css) and changing them at its sole discretion.
> **NOTE**: We recommend that you first [copy](../../library/src/styles/fiori.css) the default styles to a separate file and then modify them as you prefer.

@@ -39,9 +39,8 @@ If you make any changes in the project structure, remember to update it.

Use the following tools to develop the AsyncApi React component:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not insisting on keeping it if you think it is more of a hassle than help 🤷‍♀

Copy link
Contributor

@aerfio aerfio left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@kazydek kazydek left a comment

Choose a reason for hiding this comment

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

with a small comment to the Missing features section:

See the list of features that are still missing in the component: 

Xxx

If you want to help us develop them, feel free to contribute.

@magicmatatjahu magicmatatjahu merged commit 96dfcef into asyncapi:master Sep 11, 2019
@magicmatatjahu magicmatatjahu deleted the 2.0.0-rc2-docs branch September 11, 2019 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/community Related to all activities that are done for community area/documentation Related to all activities around documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants