Skip to content

Commit

Permalink
docs: fix typo (#3836)
Browse files Browse the repository at this point in the history
* docs: fix typo

* Create gorgeous-years-judge.md

---------

Co-authored-by: Pavithra Kodmad <pksjce@github.com>
  • Loading branch information
xiaolou86 and pksjce authored Oct 17, 2023
1 parent 32b8705 commit 038a789
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 9 deletions.
5 changes: 5 additions & 0 deletions .changeset/gorgeous-years-judge.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

docs: fix typo
12 changes: 6 additions & 6 deletions contributor-docs/adrs/adr-010-behavior-isolation.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ In both instances, a component is required to manage focus trapping, as the `foc

On top of this complexity, there's also the complexity of the behaviour itself. To properly manage focus trap, the focusTrap function creates [two sentinel elements and appends them to do the DOM](https://github.com/primer/behaviors/blob/acbcc744f56837166c2a3f76bab1f3572b61d0ca/src/focus-trap.ts#L67-L68). This increases complexity in debugging code, as there are surprising new DOM elements in the tree. This also [effects tests](https://github.com/primer/react/blob/386561a37b2b6f1f9d4b597e0ac6ede3a40ccbf7/src/__tests__/__snapshots__/AnchoredOverlay.test.tsx.snap#L222-L239). A Web Component would be able to utilise ShadowDOM to create clean separation of utility elements required by the behavior.

While `focusTrap` has been used as an example, it should be noted this is not exceptional, rather representitive. `focusZone` has a similarl [`useFocusZone` hook](https://github.com/primer/react/blob/5dd4bb1f7f92647197160298fc1f521b23b4823b/src/hooks/useFocusZone.ts), as does `anchoredPosition` with [`useAnchoredPosition`](https://github.com/primer/react/blob/5dd4bb1f7f92647197160298fc1f521b23b4823b/src/hooks/useAnchoredPosition.ts).
While `focusTrap` has been used as an example, it should be noted this is not exceptional, rather representative. `focusZone` has a similarl [`useFocusZone` hook](https://github.com/primer/react/blob/5dd4bb1f7f92647197160298fc1f521b23b4823b/src/hooks/useFocusZone.ts), as does `anchoredPosition` with [`useAnchoredPosition`](https://github.com/primer/react/blob/5dd4bb1f7f92647197160298fc1f521b23b4823b/src/hooks/useAnchoredPosition.ts).

Were these behaviours Web Components, then they would be their own container, they would have lifecycle hooks to manage internal state, and they would have a standard invocation pattern. We'd simply drop `<focus-trap active={isActive}>` into a component. The element would manage lifecycle thanks to the hooks the browser provides, and interactive state could be managed via React (or in the case of VC, another WC).

Expand All @@ -73,11 +73,11 @@ ADR 002 describes complexity around Custom Elements and the ShadowDOM. It states

Today, [ShadowDOM is very well supported](https://caniuse.com/shadowdomv1) across all modern browsers, and polyfills are not required to support GitHub Properties for ShadowDOM. GitHub.com has been using un-polyfilled ShadowDOM features for multiple years now, without issue.

Recently Primer React [added the Relative Time component](https://github.com/primer/react/pull/2484), a component that uses the ShadowDOM. This component works seamlessly with other components in the library, and does not require any additional support libraries to facilitate the use of ShadowDOM. The use of ShadowDOM within the underyling `relative-time` Custom Element allows it to update its display (a localised representation of the given time) without causing mismatches between React's internal representation of the DOM, and the DOM itself, as all updates are encapsulated away from React in the `relative-time`'s own private shadow root. This also means that React is able to do _less work_ reconciling updates to its `RelativeTime` component, which only needs to be re-rendered if the given datetime (or any other prop changes). This allows realtime updates of this component without triggering potentially costly re-renders from React.
Recently Primer React [added the Relative Time component](https://github.com/primer/react/pull/2484), a component that uses the ShadowDOM. This component works seamlessly with other components in the library, and does not require any additional support libraries to facilitate the use of ShadowDOM. The use of ShadowDOM within the underlying `relative-time` Custom Element allows it to update its display (a localised representation of the given time) without causing mismatches between React's internal representation of the DOM, and the DOM itself, as all updates are encapsulated away from React in the `relative-time`'s own private shadow root. This also means that React is able to do _less work_ reconciling updates to its `RelativeTime` component, which only needs to be re-rendered if the given datetime (or any other prop changes). This allows realtime updates of this component without triggering potentially costly re-renders from React.

#### SSR

Server Side Rendering, or "SSR" refers to construcing templates of HTML strings on the server side and delivering HTML. Custom Elements can be server rendered by specifying their tag name in the markup delivered, but they have no constraints around rendering on the server, as their behavior is entirely client side driven. Frameworks for the server such as Ruby on Rails can use "partials" or View Components to represent a custom elements markup for the server.
Server Side Rendering, or "SSR" refers to constructing templates of HTML strings on the server side and delivering HTML. Custom Elements can be server rendered by specifying their tag name in the markup delivered, but they have no constraints around rendering on the server, as their behavior is entirely client side driven. Frameworks for the server such as Ruby on Rails can use "partials" or View Components to represent a custom elements markup for the server.

React's core library does not offer a way to represent React's component tree in either the real DOM, nor on the server. In this way, developers can use either the `react-dom` library on the client, or the `react-dom/server` library using a runtime like NodeJS on the server to deliver a similar experience to traditional server side applications built in frameworks like Ruby on Rails. This is sometimes referred to as "isomorphic" code. The unique selling point of "isomorphic" components, is that developers can write a component once and have it execute in both environments, delivering a string of HTML in a server response, and reconciling DOM operations on the client.

Expand Down Expand Up @@ -136,7 +136,7 @@ For many Custom Elements, this will simply not be an issue, but these three patt

React Portals also come with their own caveats:

React Portals do not work with all elements and special consideration must already be made with regard to existing elements. For example while it is possible to create a React portal of any DOM element, elements which require a strict heirarchy such as `<details>/<summary>`, `<ul>/<li>`, `<dd>/<dt>` may cause issues if the child element is inside of a React Portal that gets moved to another part of the DOM (an [example is here](https://codepen.io/keithamus/pen/YzvMqjx?editors=0010), where you can see the `<details>` element provides a default `<summary>` of `"Details"` because the `<summary>` element has been moved).
React Portals do not work with all elements and special consideration must already be made with regard to existing elements. For example while it is possible to create a React portal of any DOM element, elements which require a strict hierarchy such as `<details>/<summary>`, `<ul>/<li>`, `<dd>/<dt>` may cause issues if the child element is inside of a React Portal that gets moved to another part of the DOM (an [example is here](https://codepen.io/keithamus/pen/YzvMqjx?editors=0010), where you can see the `<details>` element provides a default `<summary>` of `"Details"` because the `<summary>` element has been moved).

React Portals will cause the document structure to change, which also means the Accessibility tree will change. For interactive elements within a Portal, this can cause issues with focus flow, unless very large remediation strategies are in place. This limits the usability of React Portals to either non-interactive elements, or elements which focus trap.

Expand All @@ -150,7 +150,7 @@ Common in React applications is also the use of centralised state management. Ce

ADR 002 claimed that React Hooks offer better extensibility as they can interact with state which is driven by the consumer, while Custom Elements require listening to events on the document.

Given state management libraries are agnostic to the underyling framework, it is important to acknowledge that it is entirely possible to use state management libraries such as Redux or Mobx with many component frameworks such as Vue, Svelte or React, as well as with Custom Elements.
Given state management libraries are agnostic to the underlying framework, it is important to acknowledge that it is entirely possible to use state management libraries such as Redux or Mobx with many component frameworks such as Vue, Svelte or React, as well as with Custom Elements.

React components may also directly communicate with Custom Elements using the same mechanisms that React components communicate with built in elements - by passing props within JSX. A complication arises with React as it hard codes a list of attributes that map to class properties, and a list of `on*` prefixed callbacks that map to events. For example using the `hidden` prop on element rendered by React, react will look up the `hidden` property in its hard coded loookup table and map the prop to a `.hidden =` call. Props given to JSX elements which are not in this hard coded table map to `setAttribute(name)` calls, and the result is stringified.

Expand All @@ -172,7 +172,7 @@ ADR 002 enumerates concerns around development, including the issue of developin

These issues are not intrinsic to the use of shared code, however. For example sharing of code can be done within a monorepo. This is out of scope of the discussion of this ADR.

ADR 002 raises another concern around developement and contribution, with regard to familiarity of working with React and also the familiarity of working with Custom Elements, as well as the requirement for developers to context switch.
ADR 002 raises another concern around development and contribution, with regard to familiarity of working with React and also the familiarity of working with Custom Elements, as well as the requirement for developers to context switch.

The Primer View Components library can serve as an example of handling these concerns. Today, Primer View Components has been successfully integrating and authoring components using Custom Elements for multiple years.

Expand Down
6 changes: 3 additions & 3 deletions contributor-docs/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ As we heavily rely on the behavioral testing paradigm to test our components, th

- Variants of the component (I.e. default, primary, success, error)
- The states of the component (I.e. open/closed, selected, disabled)
- The look/appereance of the component
- The look/appearance of the component
- The layouts of the component
- Does component have a different layout on various viewports?
- Does component have a different layout on various states?
Expand Down Expand Up @@ -61,7 +61,7 @@ We make sure that every component has [these fundamental unit tests](https://git

### Overview

We use [Jest](https://jestjs.io/) as our test runner to write and run our unit tests. Our prefered way of structuring tests is with `describe/it` block format and they are authored within the components's source directory with the file name of `ComponentName.test.tsx`
We use [Jest](https://jestjs.io/) as our test runner to write and run our unit tests. Our preferred way of structuring tests is with `describe/it` block format and they are authored within the components's source directory with the file name of `ComponentName.test.tsx`

We predominantly use [@testing-library/react](https://testing-library.com/docs/react-testing-library/intro/)'s testing helpers such as

Expand All @@ -76,7 +76,7 @@ To make assertions about the elements we use [Jest](https://jestjs.io/) and [jes

### Snapshots

We are slowly moving away from using `Jest` snapshots as a way to test visual changes of our components. You can read more about the decision in this [ADR](https://github.com/primer/react/blob/main/contributor-docs/adrs/adr-011-snapshot-tests.md). We are in the proces of migrating our existing snapshot tests to use [Playwright](https://playwright.dev/) for visual regression testing. If you are writing a new test and you need to test the visual changes of the component, please refer to the [Visual Regression Tests](#visual-regression-tests) section.
We are slowly moving away from using `Jest` snapshots as a way to test visual changes of our components. You can read more about the decision in this [ADR](https://github.com/primer/react/blob/main/contributor-docs/adrs/adr-011-snapshot-tests.md). We are in the process of migrating our existing snapshot tests to use [Playwright](https://playwright.dev/) for visual regression testing. If you are writing a new test and you need to test the visual changes of the component, please refer to the [Visual Regression Tests](#visual-regression-tests) section.

#### Updating `theme-preval` snapshots

Expand Down

0 comments on commit 038a789

Please sign in to comment.