patternfly-react
patternfly-react copied to clipboard
patternfly
Adds content to button react examples
Makes progress on https://github.com/patternfly/patternfly-org/issues/2990
Additional issues:
Preview: https://patternfly-react-pr-7607.surge.sh
A11y report: https://patternfly-react-pr-7607-a11y.surge.sh
To go off of this comment, Matt, do you think it'd be worth including how the buttons can be disabled, e.g. "Disabled styling can be applied to any button variant with the isDisabled prop"? This would also apply to other button examples i.e. "aria disabled", "call to action" etc.
Yes, I think this sort of thing can be very helpful and was kind of what I hoped this documentation would be. It may be hard for @edonehoo to come up with all that nuance, but it would be helpful for you or another developer to point out the things that a developer should know as they look at these examples.
Again this would be a follow up issue, but @mcarrano has there been any discussion regarding examples showcasing functionality not relevant to the actual example name/description? This one for example is about "links as buttons", but includes a button that has isDisabled which may or may not be relevant. We have several components with examples that do this, so was curious if it was something we'd want to avoid doing/make a separate example for or if it isn't really a huge deal.
I also see this process as one of making out examples more relevant and meaningful as between the live examples and the documentation, it should tell a story of what this component is and how to use it
To go off of this comment, Matt, do you think it'd be worth including how the buttons can be disabled, e.g. "Disabled styling can be applied to any button variant with the isDisabled prop"? This would also apply to other button examples i.e. "aria disabled", "call to action" etc.
Yes, I think this sort of thing can be very helpful and was kind of what I hoped this documentation would be. It may be hard for @edonehoo to come up with all that nuance, but it would be helpful for you or another developer to point out the things that a developer should know as they look at these examples.
Appreciate all the feedback so far. A lot of everyone's insight related directly to the questions I was planning to bring up in Thursday's check-in, so this has been helpful! Working on integrating all of this into an update.
@mcarrano I agree that many of @thatblindgeye technical add-ons make a lot of sense to include. It does lead me to wonder about the most efficient way that I can draft these edits without making it too tedious for engineers to have to come in and add nuance or functionality that I'm not aware of. For example, I tried to understand the aria stuff on my own and ended up being a little off. & then when it comes to the control variant (which oops I didn't notice on PF), tooltip support, etc. -- there are things I struggle to find context on entirely. I definitely don't mind taking a swing and missing, but I'm trying to be cognizant of assuming too much. Ultimately I'm just hoping it's not too troublesome to review!
@edonehoo this looks great so far, thanks for tackling this ! I don't think you need to be an expert with the technical stuff, just having someone sit down and take the time to add this text is really helpful and gets the ball rolling, so don't worry about the review work too much - the devs are here to suggest technical feedback haha!
@mcarrano I honestly wouldn't worry too much about repetition with the design guidelines, especially because I think a lot of people might only look at one or the other (?) I don't think this is worth re-doing the whole structure of our design guidelines over, personally!
Yes, I think this sort of thing can be very helpful and was kind of what I hoped this documentation would be. It may be hard for @edonehoo to come up with all that nuance, but it would be helpful for you or another developer to point out the things that a developer should know as they look at these examples.
Perfect, sounds good! Wanted to be sure whether that sort of info would be worth including the description first, but will definitely keep that in mind when reviewing future example work as well.
It does lead me to wonder about the most efficient way that I can draft these edits without making it too tedious for engineers to have to come in and add nuance or functionality that I'm not aware of. For example, I tried to understand the aria stuff on my own and ended up being a little off. & then when it comes to the control variant (which oops I didn't notice on PF), tooltip support, etc. -- there are things I struggle to find context on entirely. I definitely don't mind taking a swing and missing, but I'm trying to be cognizant of assuming too much. Ultimately I'm just hoping it's not too troublesome to review!
@edonehoo totally agree with Margot, the work you've done already has been great to get things rolling! I also agree in not worrying too much about the technical stuff, but it's also awesome you've tried looking into some of the more technical stuff on your own. If there's ever anything in particular you aren't too sure about you could either mention it in the initial PR comment or you can reach out to Katie or myself (or another dev!), otherwise it's definitely not tedious to review and add comments for that sort of stuff from the dev perspective 🙂
@mcarrano @evwilkin @thatblindgeye @kmcfaul @mmenestr + etc - I incorporated previous feedback in my latest commit and also took some liberties following today's discussion (rearranged things and messed with headings). Happy to roll back those bigger changes if they're an issue or don't work how I intend, but I think they help visualize what I had in mind. Made content changes too so please lmk if there are typos/misinformation/etc!
RE: "Also, there seems to be a stray heading labeled "Untitled example" at the head of the file when I preview it. But I can't see where that comes from in the source file." -- so I removed the existing Examples header because it seemed redundant, but maybe it's something that is required to be at the top of the file? Does anyone know if that's the case? If so, I can just add it back (I don't see the stray header, but I may be overlooking it)!
@edonehoo
It seems that the website is only configured to handle heading levels up to h3 - so titles with only 3 ### before them.
I think that's why it isn't rendering correctly.
