feat: add accessible react-bootstrap components #550

abutterworth · 2020-07-15T17:05:40Z

Adds the following react-bootstrap components with jump off documentation pages:

Alert
Badge
Button Group
Card
Carousel
Figure
Forms
Input Group
Image
Navs
Navbar
Overlays
Popover
ProgressBar
Spinner
Tooltip
Toast

Sample Doc:

coveralls · 2020-07-15T17:20:16Z

Coverage remained the same at 88.012% when pulling e957307 on abutterworth/add-react-bootstrap into c9c2632 on master.

Skip conflicting components that will be shimmed. Add TODOs for components with a11y defects to be addressed in the future

davidjoy · 2020-07-16T13:35:38Z

src/index.js

+export { default as BreadcrumbItem } from 'react-bootstrap/BreadcrumbItem';
+*/
+// TODO: create shim – export { default as Button } from 'react-bootstrap/Button';
+export { default as ButtonGroup } from 'react-bootstrap/ButtonGroup';


Are these Button* components compatible with our Button above, that we have yet to shim?

They are! I tested them in the doc page just added.

davidjoy · 2020-07-16T13:40:03Z

Two thoughts -

One, I'm curious how a lot of these interact with our existing Paragon components when they're used in sensible ways that folks might expect (our input classes inside their Form, for instance)

Two, documenting all this will be interesting. In the past we chose to duplicate/wrap all the Bootstrap documentation... are we going to reproduce react-bootstrap's? Or just reference it from the paragon documentation? Seems like we're going to need to have a big shift in the doc around which components are deprecated... are we doing a major version change and just removing stuff? And in reference to the first thought above, documenting any subtlety to those interactions will be fun.

davidjoy · 2020-07-16T13:40:32Z

And also, just a reminder to check on tree-shaking to make sure they've packaged react-bootstrap in such a way that it can still be slimmed down when used through Paragon.

davidjoy · 2020-07-16T13:41:49Z

Also - after having investigated react-bootstrap quite a bit now, how many cases do we see where we want to 'wrap' things? Have we seen any specific benefits to exposing them through paragon?

Just thinking about whether or not it'd be a reasonable choice to make react-bootstrap a recommended library and call it a day. 🤔 (not sure I think this is a good idea)

abutterworth · 2020-07-16T14:02:21Z

Great points David.

Interaction between components: So far it seems like they're pretty interoperable. I know ValidationFormGroup will not do its job with the react-bootstrap components. I intend to mark the components that I know won't work together. As far as I'm aware right now, that's the only one. I may find more as I continue building the docs.
Documentation: I'm adding one or two examples of usage and then linking to the react-bootstrap docs themselves. I think this is the best blend between convenience and not doing extra work
Tree-shaking: I will double check in a project, but judging by the structure I'm confident it will tree-shake fine.

Why pull it in? Incorporating react-bootstrap inside paragon will help us:

maintain flexibility in the future by making Paragon the single source of truth. We can replace react-bootstrap components with our own in the future if we choose.
By using Paragon to pull in these components we can leverage them internally in Paragon without worrying about peer deps or versions of react-bootstrap.

In general I expect the future of Paragon to aim to create more "molecule-level" components that are composed of these react-bootstrap elements and in the further future we can swap the underlying technology with CSS-in-JS iteratively.

stvstnfrd

Just a question and a typo (?) in my drive-by review.

Just want to say, I'm really glad to see paragon coming together; a unified style/component guide/manual is huuuge and long-awaited!

src/index.js

davidjoy · 2020-07-17T13:54:01Z

Thanks for indulging the second-guessing questions, @abutterworth. Explanations totally make sense/I'm behind this!

davidjoy · 2020-07-17T13:54:30Z

src/index.js

@@ -1,10 +1,10 @@
 export { default as asInput } from './asInput';
 export { default as Breadcrumb } from './Breadcrumb';
-export { default as Button } from './Button';
+export { default as Button } from './Button'; // TODO: shim from react-bootstrap


Are these TODOs something that should be completed in this PR? Or for later?

For follow up PRs

Adding the shims is more risky in terms of inadvertently making breaking changes so I'll keep them separate.

davidjoy · 2020-07-17T13:58:08Z

www/src/pages/components/statusalert.mdx

@@ -1,13 +1,11 @@
 ---
 title: 'StatusAlert'
 type: 'component'
-status: 'Needs Work'
+status: 'Deprecate Soon'


Feel like we have so many variations on this alert floating around in the MFEs. :)

adamstankiewicz · 2020-07-17T13:24:44Z

src/index.js

+export { default as Overlay } from 'react-bootstrap/Overlay';
+export { default as OverlayTrigger } from 'react-bootstrap/OverlayTrigger';
+export { default as PageItem } from 'react-bootstrap/PageItem';
+// TODO: create shim – export { default as Pagination } from 'react-bootstrap/Pagination';


We won't be able to simply shim react-bootstrap's Pagination component here, since the component just provides the visual UI, without the underlying logic to actually make it function, where it sends callbacks and accepts props such as current page, onPageSelect, etc.

By shimming it directly, the consumers on the Pagination component would need to implement that logic themselves.

Good call out, with Pagination in particular. What do you think about a future breaking change that pulls in react-bootstrap's pagination and moving the purpose of the current Pagination to a PaginationGroup or something?

adamstankiewicz · 2020-07-17T13:35:06Z

www/src/pages/components/image.mdx

+      <Image src={generateCustomPlaceholderURL(180, 180)} roundedCircle />
+    </Col>
+    <Col xs={6} md={4}>
+      <Image src={generateCustomPlaceholderURL(180, 180)} thumbnail />


nit: might be good to include the fluid prop on one of these examples, as I imagine that will be a commonly used prop for Image.

I was trying to figure out what fluid actually does and couldn't see any change there. What's it for?

edx-semantic-release · 2020-07-17T15:48:15Z

🎉 This PR is included in version 9.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

abutterworth changed the title ~~feat: add react-bootstrap alert~~ [WIP] feat: add react-bootstrap alert Jul 15, 2020

abutterworth changed the title ~~[WIP] feat: add react-bootstrap alert~~ [WIP] feat: add accessible react-bootstrap components Jul 15, 2020

feat: add react-bootstrap components

7b88a66

Skip conflicting components that will be shimmed. Add TODOs for components with a11y defects to be addressed in the future

abutterworth force-pushed the abutterworth/add-react-bootstrap branch from 8b86f77 to 7b88a66 Compare July 16, 2020 13:16

docs: add links to new to-be-added component pages

a03aec7

davidjoy reviewed Jul 16, 2020

View reviewed changes

docs: add docs for four components

3b1c192

Adam Butterworth added 17 commits July 16, 2020 10:21

docs: add doc for carousel

a90902c

docs: add forms documentation

59a4104

docs: add doc for input group

47605d5

docs: add doc for image

b0d90ed

docs: add docs for figure

5b0e6e3

docs: add doc for jumbotron

9d1402f

docs: add doc for navs

87ac991

docs: add docs for navbar

a8fded8

docs: add doc for overlay

b80aac1

docs: add doc for popover

e4229c7

docs: add doc for progress bar

d23cba9

docs: add doc for spinner

0886bc7

docs: add doc for tooltip

5fa4f35

docs: add doc for toast

e52f4d3

docs: update statuses

2cfb130

fix: remove jumbotron

d73540b

docs: add stub doc for accordion

166233a

abutterworth changed the title ~~[WIP] feat: add accessible react-bootstrap components~~ feat: add accessible react-bootstrap components Jul 16, 2020

abutterworth requested review from davidjoy and a team July 16, 2020 21:30

stvstnfrd reviewed Jul 17, 2020

View reviewed changes

src/index.js Outdated Show resolved Hide resolved

docs: fix typo

7efb476

davidjoy reviewed Jul 17, 2020

View reviewed changes

davidjoy approved these changes Jul 17, 2020

View reviewed changes

adamstankiewicz reviewed Jul 17, 2020

View reviewed changes

adamstankiewicz approved these changes Jul 17, 2020

View reviewed changes

docs: swap placeholder images with unsplash urls

e957307

abutterworth merged commit b067fb4 into master Jul 17, 2020

abutterworth deleted the abutterworth/add-react-bootstrap branch July 17, 2020 15:44

edx-semantic-release added the released label Jul 17, 2020

abutterworth mentioned this pull request Feb 26, 2021

Refactor asInput #229

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add accessible react-bootstrap components #550

feat: add accessible react-bootstrap components #550

abutterworth commented Jul 15, 2020 •

edited

coveralls commented Jul 15, 2020 •

edited

davidjoy Jul 16, 2020

abutterworth Jul 16, 2020

davidjoy commented Jul 16, 2020

davidjoy commented Jul 16, 2020

davidjoy commented Jul 16, 2020

abutterworth commented Jul 16, 2020

stvstnfrd left a comment

davidjoy commented Jul 17, 2020

davidjoy Jul 17, 2020

abutterworth Jul 17, 2020

abutterworth Jul 17, 2020

davidjoy Jul 17, 2020

adamstankiewicz Jul 17, 2020

abutterworth Jul 17, 2020

adamstankiewicz Jul 17, 2020

abutterworth Jul 17, 2020

edx-semantic-release commented Jul 17, 2020

feat: add accessible react-bootstrap components #550

feat: add accessible react-bootstrap components #550

Conversation

abutterworth commented Jul 15, 2020 • edited

coveralls commented Jul 15, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidjoy commented Jul 16, 2020

davidjoy commented Jul 16, 2020

davidjoy commented Jul 16, 2020

abutterworth commented Jul 16, 2020

stvstnfrd left a comment

Choose a reason for hiding this comment

davidjoy commented Jul 17, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edx-semantic-release commented Jul 17, 2020

abutterworth commented Jul 15, 2020 •

edited

coveralls commented Jul 15, 2020 •

edited