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.

Sign up for GitHub

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

added a Text component #18495

Merged
merged 24 commits into from
Jan 7, 2020
Merged

added a Text component #18495

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
b804e72
added a Text component
Nov 13, 2019
de80cdc
avoid exporting and documenting the mixin
Nov 14, 2019
d3325eb
fix storybook title
Nov 14, 2019
56cc6cb
use kebab-case for file naming
Nov 14, 2019
f8e5d14
added a _default story with all variants in the one story
Nov 14, 2019
1dcff30
update styles
Nov 14, 2019
726aadd
update snapshots for react-native
Nov 14, 2019
5943ded
git and case-sensitive renames are the worst on macos
Nov 14, 2019
408a907
git and case-sensitive renames are the worst on macos
Nov 14, 2019
8d0d288
rename tests in kebab-case
Nov 14, 2019
8a87ff8
renamed snapshot files
Nov 17, 2019
1fe6f2f
d'oh forgot to fix the casing ina file
Nov 18, 2019
57e9542
update the readme to include when to use the component
Nov 24, 2019
10853c3
removed eslint disable rule
Nov 25, 2019
58e0ed2
update native snapshots
Nov 25, 2019
1d4ace8
delete unnecessary tests ready for visual regression testing
Nov 28, 2019
de00cb5
update storyshots
Nov 28, 2019
6d46659
commit updates to generated file
Dec 6, 2019
e659e10
renamed Text to __experimentalText
Dec 10, 2019
eee98ce
remove unnecessary files
Dec 13, 2019
3fdd23f
fix formatting
Dec 13, 2019
0967a4d
Update formatting of typings
jameslnewell Dec 13, 2019
b0d4d55
Mark stories as experimental
jameslnewell Dec 13, 2019
614a8ab
updated story snapshots
Dec 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/manifest-devhub.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -983,6 +983,12 @@
"markdown_source": "../packages/components/src/text-highlight/README.md",
"parent": "components"
},
{
"title": "Text",
"slug": "text",
"markdown_source": "../packages/components/src/text/README.md",
"parent": "components"
},
{
"title": "TextareaControl",
"slug": "textarea-control",
Expand Down
116 changes: 100 additions & 16 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@
"husky": "3.0.5",
"inquirer": "6.3.1",
"is-equal-shallow": "0.1.3",
"jest-emotion": "10.0.17",
jameslnewell marked this conversation as resolved.
Show resolved Hide resolved
"jest-junit": "6.4.0",
"jest-serializer-enzyme": "1.0.0",
"jsdom": "11.12.0",
Expand Down
6 changes: 4 additions & 2 deletions packages/components/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@
"sideEffects": false,
"dependencies": {
"@babel/runtime": "^7.4.4",
"@emotion/core": "10.0.22",
"@emotion/styled": "10.0.23",
"@emotion/core": "^10.0.22",
"@emotion/css": "^10.0.22",
"@emotion/native": "^10.0.22",
jameslnewell marked this conversation as resolved.
Show resolved Hide resolved
"@emotion/styled": "^10.0.23",
"@wordpress/a11y": "file:../a11y",
"@wordpress/compose": "file:../compose",
"@wordpress/deprecated": "file:../deprecated",
Expand Down
1 change: 1 addition & 0 deletions packages/components/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,3 +104,4 @@ export { default as withNotices } from './higher-order/with-notices';
export {
default as withSpokenMessages,
} from './higher-order/with-spoken-messages';
export * from './text';
jameslnewell marked this conversation as resolved.
Show resolved Hide resolved
18 changes: 18 additions & 0 deletions packages/components/src/text/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Text

A text component for styling text.

## Usage

```jsx
import {Text} from '@wordpress/components';

const HeroPanel = () => (
<>
<Text variant="title.large" as="h1">Hello World!</Text>
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the title.large option default to h1? In general, it would be great to have this more opinionated so all popular HTML tags are covered with variants. as can be still used to customize it if necessary.

Copy link
Contributor Author

@jameslnewell jameslnewell Nov 26, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

We could potentially do that. Is the location where title.large usually gets used usually a h1 too? (not just thinking about Gutenberg but thinking about other plugins too)

If we change the default element for one variant, should we change the default element for the other variants too?

Copy link
Member

Choose a reason for hiding this comment

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

If we change the default element for one variant, should we change the default element for the other variants too?

Yes, this is what I meant by saying – it would be great to have this more opinionated so all popular HTML tags are covered with variants. I assume that the whole idea is to stop using HTML tags, so this should make it easier to keep the code platform agnostic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍I'll let @davewhitley jump in to discuss whether there's a clear link between each variant and and a semantic element.

Copy link
Contributor

@davewhitley davewhitley Nov 27, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

It wasn't designed with a heading level in mind, no. I'm hesitant to assign each heading level to a typographic style, because they should be separate. Each typographic style can use whatever HTML element is appropriate.

If we default to a p or h2 element for each style we are forcing the user of the component to make a choice on which heading level element to use, which is a good thing imo.

I'm afraid that eventually people will just associate h1 for Title Large, just as an example.

I don't know enough about the Text component or how/when it should be used to give an authoritative answer though. I do know that I'm concerned about people ignoring the heading level and just using the default value, which will result in inaccessible markup. I'm also concerned about people choosing a specific typographic style because of the heading level associated with it. E.g. "I'll choose Title Medium because I need a level 2 heading". It’s important to realize that the style of a heading is independent from the semantic underlying element.

Maybe look at Material UI or Polaris to see how it's done?

Copy link
Contributor Author

@jameslnewell jameslnewell Nov 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure Polaris is helpful...

The Display Text variants (kind of like our title.* variants) maps to a p.
The Heading variant which offers a single size maps to a h2.
The Subtitle variant which offers a single size maps to a h3.
Everything other variant maps to a p.

I'd advocate that we don't be overly opinionated by defaulting to anything other than a p at this time and wait and see how the component gets used and whether there are any consistent usage patterns that form and can be implemented into the component.

<Text variant="body">Greetings to you!</Text>
</>
);
```

> For most use-cases you can use this component instead of a `h1`, `h2`, `h3`, `h4`, `h5`, `h6` or `p`.
2 changes: 2 additions & 0 deletions packages/components/src/text/font-family.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export const fontFamily = `font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto,
Oxygen-Sans, Ubuntu, Cantarell, "Helvetica Neue", sans-serif;`;
1 change: 1 addition & 0 deletions packages/components/src/text/font-family.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const fontFamily = '';
1 change: 1 addition & 0 deletions packages/components/src/text/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './text.styles';
Loading