primer / react Goto Github PK
View Code? Open in Web Editor NEWAn implementation of GitHub's Primer Design System using React
Home Page: https://primer.style/guides/react
License: MIT License
An implementation of GitHub's Primer Design System using React
Home Page: https://primer.style/guides/react
License: MIT License
The name map()
is so ambiguous. Let's make it clearer!
Ref: #70 (comment)
This can just be a pr off of Primer for now since we automatically publish alpha releases.
I forgot to update after changing the purpose of our build
run-script. ๐ Ref: #96 (comment)
Last week I went through the rest of the components in Primer and noted which ones are missing from primer-react:
You can't import the latest version of primer-react
because the published package doesn't include the dist/
directory. This is because I put the build step in our preversion
run-script instead of prepublishOnly
, and since we're not versioning on Travis it never ran. So yeah, I need to fix that. ๐คฆโโ๏ธ
We need to come up with a name for a prop that describes a color theme (background color + text color, etc)... theme
could easily be mixed up with our themes (light/dark). Maybe colorPattern
? colorScheme
? maybe theme
is ok? thoughts?
Items that need to be completed before our first public beta
primer/components
The TextInput needs an onChange handler in order to handle changes to the input
We need some "official" way to import Primer CSS. Here's how we've done it in our docs site:
unpkg.com
URLs. This is okay for demos and sandboxes, but not good for production.Currently on the static site the top level nav goes to urls like this /primer-react/docs/[page]
, but clicking on components in the component library changes the url to /docs/[page]/[component]
. This is due to a difference between how routing is working locally vs on the static site. Locally, /primer-react
is not prefixed to every url (because GH pages is doing that), so in order for HMR to work, the routes need to go to /docs/page/component
instead of /primer-react/docs/page/component
. Fun times ๐
The url generated by gh pages is /primer-react
, but the static site actually lives at /primer-react/docs
@shawnbot noticed that the TextInput component include an autofocus
prop + attribute. I'm curious if there's a strong use case for needing this here, or if it would be ok to remove it? The reason being that autofocus
can have negative a11y consequences as it disorients screen reader users without warning. From MDN:
Warning: Automatically focusing a form control can confuse visually-impaired people who using screen-reading technology. When autofocus is assigned, screen-readers "teleport" their user to the form control without warning them beforehand.
Additionally, only one element per page should have the autofocus
attribute and we currently don't have any way of enforcing that.
Curious if there is a strong reason why we decided to add it? cc @jonrohan @shawnbot @broccolini
We need Primer-specific <details>
+ <summary>
components for #45 (and #43):
This one is a slightly more complicated version of a native <details>
, in that it needs to show different text depending on when it's open or closed.
In this one, the "pressed" state of the arrow changes when you open and close it, and the dropdown content is absolutely positioned. The tricky thing here is using <details>
and <summary>
, because they require that the latter is nested directly in the former.
I've whipped up an example of how we could do this using the render props pattern, which makes the open
and toggle
props available to children so that the author can decide what's visible and hidden in the summary without managing state.
Some setups we might want to test with:
Ref: #70 (comment)
What do you think about changing the
shadow
API here to acceptsmall
,medium
,large
,extra-large
and then instead of accepting either a boolean or a string we can just accept strings and this line would beshadow && ((shadow === 'small') ? 'box-shadow' : `box-shadow-${shadow}`)
Example of a styled-component as a primitive, extended, and as a composite component.
child.props.key
doesn't exist, which results in un-keyed elements. Maybe calling React.cloneElement()
would be better here?DonutPropType
bit is kind of silly. That PropTypes.shape()
call can be wrapped in oneOrMoreOf()
from props.js
.This week's work has given us an opportunity to mull over whether components should be "strict" in which props they allow users to pass โ or, as an implementation detail, which ones they actually respect. I can think of a couple that we may want to whitelist for some, if not all, Primer components:
id
could people to provide deep links without having to wrap another element around their content.hidden
would provide a more systematic way to show and hide things.aria-
attribute, because a11y.data-
attribute, to provide hooks for JS behaviors implemented by container components.Thoughts?
When publishing Kit to gh pages, permalinks are broken
Needs to include API for:
We should run a npm vulnerability audit on our packages, I just upgraded to npm 6 which has this feature.
In primer/primer
we have a separate package for each component or group of styles. We set it up like this so that people could only pull in the styles they need so as to avoid unnecessary bloat. Do we need to do this with React components? We don't have quite the same concerns with CSS bloat. I do think we will still have some distinction between dotcom and marketing sites, so perhaps there is an "addons" package, but otherwise maybe we don't need to break it up quite as much. Let me know what you think!
cc @primer/ds-core
Add:
In use this might look like <Icon alert />
.
This article seems to provide a good approach: http://nicolasgallagher.com/making-svg-icon-libraries-for-react-apps/
@emplums inquired in #28 (comment) which jsx-a11y
rules eslint-plugin-github uses, which led me to this massive hunk. Emily suggested that we adopt the jsx-a11y/recommended
settings instead, which prompted the question:
Why aren't we using the recommended rules in eslint-plugin-github
?
Maybe @josh can fill us in. :)
I did some testing using primer-react in a project this weekend, and I found some things too inflexible in places that will make it too hard to work with and might cause us some headaches too. I think I'd rather be a little more flexible than constrain stuff too much. Here's some suggestions:
space
(margin/padding) and color
(color/bg) utility propsI think we can group components in categories to help us make the same utility props available, this will make them easier to compose with, and create some standardization across components:
fontSize
, lineHeight
Text
, Heading
, Link
, Label
, Counter
, State Tooltip
, Button, Form elementswidth
(such as width={[1,2/3, 1/2]}
mapping to our col-width utilities), container
(such as container-lg
) (same as breakpoints), border
onClick
handlers
Some other areas that need more thought:
cc @primer/ds-core
The fontSize prop in Text goes from 0-6 and maps them backwards to match up to the primer/primer scale which goes from 6-0. Let's create 1:1 with primer/primer and keep the scale going from 6-0
I think it makes sense to fork this repo and make a version of react components that uses existing Primer styles only. That allows us to explore how we break down components while we test out CSS-in-JS solution which will take longer to build.
Such as the Details toggle.
Currently our process of running npm run build
to update the docs
directory (so that we can then publish to GitHub Pages) is a bit tedious and error-prone because:
I propose that we build the site on CI (which also ensures that the site always builds) and publish it from there, ideally to a branch-specific URL on the gh-pages
branch so that the URL would be, for instance, https://primer.github.io/primer-react/preview/my-branch-name/
.
From @emplums in this review:
This is maybe one for another PR, but what do you think about separating the border utilities into two different props? We could have
border
andborderColor
? I think that would make reasoning with the prop a bit more intuitive on the user side & would make it easier to create classes here! It looks like the styleguide docs also encourage using two separate classes for border/border-top/etc and border color utilities
Also curious if we need to allow people to pass in
border={0}
, and we instead make the Block component have no border by default?
I agree 100%!
I've been using the Primer mapping from system-classnames
in #32, but after looking into Text and Heading components I think we should consider more granular mappings for each, e.g.
import createMapper from 'system-classnames'
const breakpoints = [null, 'sm', 'md', 'lg', 'xl']
// one day? import {breakpoints} from 'primer-primitives'
const createPrimerMapper = props => createMapper({
breakpoints,
props,
getter: ({breakpoint, prop, value}) => breakpoint
? [prop, breakpoint, value].join('-')
: [prop, value].join('-')
})
const textMap = createPrimerMapper(['h', 'f', 'lh', 'text'])
const boxMap = createPrimerMapper([
'bg', 'text', 'border', 'rounded', 'box-shadow',
'position', 'top', 'right', 'bottom', 'left',
'v-align', 'overflow', /* 'float', */
'width', 'height', 'min-width' // these might need to be handled separately
])
export default createPrimerMapper
export {textMap, boxMap}
Then Box, Text, and Heading would each use them like:
const Box = props => <div {...boxMap(props)} />
const Text = props => <span {...textMap(props)} />
const Heading = props => <h1 {...textMap(props)} />
Heading.h2 = props => <h2 {...textMap(props)} />
// etc.
Does this seem like the right way to go about it?
Our x0-driven docs site doesn't generate working component permalinks. When we publish to Pages, the index loads from /primer-react/
, but the links all go to /{ComponentName}
(without the right path prefix). The pages for individual components don't even really exist (there's only an index.html
in the docs
directory), so I'm not sure what the URLs really should be for a component example. Having working permalinks seems pretty crucial to me.
To reproduce, visit https://primer.github.io/primer-react/
and click on "Details". The URL changes to https://primer.github.io/Details
, which doesn't exist. But neither do any of the following:
https://primer.github.io/primer-react/Details
https://primer.github.io/primer-react/?Details
https://primer.github.io/primer-react/#Details
We've already set x0.basename
in package.json
as described in x0's docs, but the URLs that are getting passed to location.replace()
or history.pushState()
just aren't right.
Any ideas, @broccolini?
Our UMD build doesn't currently bundle any of the external dependencies, including d3-shape
or Octicons. If we want to support CodePen and other, dependency-unaware environments, we'll need to build a browser-ready UMD bundle with all the batteries included.
Extend a component (such as buttons) to provide an example of how we'll approach this with any component, such as when do you extend vs add props.
To act as a template for other composit components. I.e. show how we handle spacing and styling components within a component etc.
The Caret component introduced in #58 has some issues that I'd like to address:
edge
and align
props is kind of confusing, and the align
prop is currently ignored. I think that a location
prop with values like top
, top-left
, etc. will be more intuitive, and matches the Popover position naming conventions.We should take this opportunity to address the positioning inconsistencies with Popover, and possibly even re-implement Popover as a Box + Caret!
@emplums asked a good question here:
[...] we are inverting the fontSize scale here?
When @broccolini started this repo, fontSize
was translated directly to CSS font-size
in a styled component. So my goal was to have a prop that mapped increasing numbers to increasing sizes, which is the inverse of our f?
classes. ๐ฑ I have no idea which is "right", though, so let's discuss!
rendersClass
into utility fileConsider using @muan's work on web component / react menus https://github.com/github/devtools/pull/226
https://github.com/github/details-menu-element
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.