There are quite a few GitHub Actions in this repo that uses node version 16 and its support is EOL

GitHub Actions list that uses nodejs 16: https://github.com/search?q=repo%3Aprimer%2Fbehaviors%20node-version&type=code

We need to update these actions to use higher, possibly latest Node versions.

Elements with contenteditable=true are focusable natively in the browser. We would like for iterateFocusableElements method to also account for contenteditable when checking for focusable elements

MDN: https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/contenteditable

Snippet in iterate-focusable-elements.ts where focus is checked: https://github.com/primer/behaviors/blob/main/src/utils/iterate-focusable-elements.ts#L86

NOTE: For lack of a better term here, when I refer to detached focus I mean when no specific element is focused but the browsers knows where in the focus flow the user should be. For example, this can happen when the currently focused element is unmounted or disabled, or if the element is the first focuseable item on the page.

When tabbing from a detached focus state to a focus zone that has a focusInStrategy function set, the function is not called and thus the first focuseable element in the zone is always focused. This appears to be because the function is only called if event.relatedTarget instanceof HTMLElement but there is no relatedTarget in this case (relatedTarget is null). It looks like the check may just be here to satisfy TypeScript:

Here is a minimal demo of an instance where relatedTarget can be null: https://jsfiddle.net/9cn7zxaq/

Here is an example of an instance where this causes a problem. I am building a datepicker and when I press the "Today" button, it is disabled because the picker moves to today's date. Focus is lost, then if the user Shift+Tabs to the days grid, I have a focusInStrategy that will set the focus to the correct day. However, my strategy is not called and the focus is set to the first day of the month instead.

Hi, I noticed that the tooltips in PR pages are not aligned properly when body has margin set.

Check out the comparison below, you can see the tooltip shifts 200px relative to body between the 2 cases. And 200px is exactly the margin-left set to body.

no margin on body	Manually add 200px margin on body

Why would body has margin?

I noticed this issue because I use a GitHub browser extension that adds a sidebar to the page and set margin to body so that the sidebar would not overlap page content. I understand that primer is not responsible for handling of user's browser extension use case. But perhaps my use case has revealed a tiny defect of primer/behaviors and would help improve it. :)

Some insights I have noticed

Tooltip's position was set by

https://github.com/primer/view_components/blob/81eea01d0034c55771ce3e6c2afc685d364afc6c/app/components/primer/alpha/tool_tip.ts#L416-L417

And the position was calculated from getAnchoredPosition

In getAnchoredPosition, the relativeRect has the correct left value for tooltip, if it was set to tooltip's left style, it would align properly.

However, the relativeRect was then passed to pureCalculateAnchoredPosition and it returned a different left.

Thank you for this awesome project!

Moving focus stops working upwards if hidden attribute is set to true of an element in the focus flow

To reproduce:

I have button that I hide when it is clicked

Start moving focus downwards till button is reached, click that button, click handler will execute button.hidden = true;
moving focus downwards if further elements are present below the now hidden button works
moving focus upwards from where the button was previously displayed no longer works

So setting an element to hidden in the chain, disrupts the focus flow upwards starting from that element

Ignore please!

related - primer/react#2454

Description

This issue may better exist on primer/behaviors (or even upstreamed to jsdom if we can repro simply enough)

Initially reported in slack by @itsbagpack
https://github.slack.com/archives/C01L618AEP9/p1666186967969609

👋:skin-tone-2: hi friends, i’m running into some testing issues around using the Autocomplete component and i was wondering what we were missing
for context, here’s the PR we’re working with: https://github.com/github/delorean/pull/127

TypeError: Failed to execute 'addEventListener' on 'EventTarget': parameter 3 dictionary has member 'signal' that is not of type 'AbortSignal'.
❯ exports.convert node_modules/jsdom/lib/jsdom/living/generated/AddEventListenerOptions.js:53:11
❯ HTMLDivElement.addEventListener node_modules/jsdom/lib/jsdom/living/generated/EventTarget.js:139:46
❯ Module.focusZone node_modules/@primer/behaviors/dist/esm/focus-zone.js:248:15
   246|     });
   247|     let elementIndexFocusedByClick = undefined;
   248|     container.addEventListener('mousedown', event => {
      |               ^
   249|         if (event.target instanceof HTMLElement && event.target !== document.activeElement) {
   250|             elementIndexFocusedByClick = focusableElements.indexOf(event.target);
❯ node_modules/@primer/react/lib-esm/hooks/useFocusZone.js:21:34

It appears that jsdom is throwing when trying to add an event listener, and appears that's due to the primer/behaviors focusZone attempting to pass a signal, which appears to conflict with the accepted signal jsdom expects.

Primer/react uses jest (with jsdom) to test this component, which is confusing to me, given there's no indication that it currently has issues here

This leads me to think either
a version of JSDOM begins breaking
vite and jest somehow differ in the globals they send to jsdom in this scenario

I suggested a workaround, temporarily, mocking focusZone entirely -

vi.mock("@primer/behaviors", async () => {
 const behaviors = await vi.importActual<typeof import("@primer/behaviors")>(
   "@primer/behaviors",
 )

 return {
   ...behaviors,
   focusZone: vi.fn(),
 }
})

but this changes behavior of the autocomplete, and is not ideal

Steps to reproduce

start a project with vite/vitest
render an AutoComplete in a test using the jsdom environment

see jsdom throw

Version

All related versions of multiple packages

It's unclear to me whether this problem already exists and is becoming clear due to versioning of react and testing-libary, is due to something with how vite interops globals compared to jest, or if this is jest/jsdom.

"dependencies": {
    "@primer/octicons-react": "^17.7.0",
    "@primer/react": "^35.11.0-rc.65a8bb8a",
    "react": "^18.2.0",
    "react-dom": "^18.2.0",
    "styled-components": "^5.3.5"
  },
  "devDependencies": {
    "@testing-library/jest-dom": "^5.16.5",
    "@testing-library/react": "^13.4.0",
    "@testing-library/user-event": "^14.4.3",
    "@types/react": "^18.0.17",
    "@types/react-dom": "^18.0.6",
    "@vitejs/plugin-react": "^2.0.1",
    "typescript": "^4.6.4",
    "vite": "^3.0.7",
    "vitest": "^0.23.4"
  }

Browser

No response

• via primer/react#2270

Describe the bug
If an element is given a tabIndex of -1 and filtered out of a Focus Zone with focusableElementFilter, it should not participate in the Focus Zone or be tabbable. This works as expected until some prop on the unfocusable element changes. When that happens, the Focus Zone will update the tabIndex of that element to be 0, making it tabbable which not desired. This seems like an edge case, but it is occurring in the ReposFileTree component, which contains expand/collapse buttons in each row that should not participate in the Focus Zone.

Note: As this is a public repo, please be careful not to include details or screenshots from unreleased GitHub products or features. In most cases, a good bug report should be able to describe the new component without including business logic or feature details, but if you must discuss context relating to an unreleased feature, please open an issue in the private Design Systems repo and link to it here.

To Reproduce
Here's a simple example:

export const DefaultButton = (args: ButtonProps) => {
  const [expanded, setExpanded] = useState(false)
  const {containerRef} = useFocusZone({
    focusableElementFilter: element => element.getAttribute('data-focusable') !== 'false'
  })
  return (
    <Box ref={containerRef as RefObject<HTMLDivElement>}>
      <Button {...args}>Default</Button>
      <Button {...args}>Default2</Button>
      <Button
        {...args}
        data-focusable="false"
        tabIndex={-1}
        aria-expanded={expanded}
        onClick={() => {
          setExpanded(!expanded)
        }}
      >
        Default3
      </Button>
      <Button {...args}>Default4</Button>
    </Box>
  )
}

1. Paste into the DefaultButton storyboook example in this repo
2. Inspect the Default3 Button and notice it has a tabIndex of -1
3. Click on the Default3 Button and notice its tabIndex changes to 0

Expected behavior
Elements that are being filtered out of the focus zone with focusableElementFilter should not have their tabIndex updated by it.

Desktop (please complete the following information):

• OS: MacOS
• Browser chrome
• Version 103.0.5060.134 (Official Build) (x86_64)

Additional context
Add any other context about the problem here.

We need to set up real docs for this project. We can pull the existing docs out of PRC and add a new "Behaviors" section to primer.styles

w/ help from @colebemis

For ArrowVertical does not take into account input elements of type "number" that have step defined, meaning it overrides the default behaviour making stepping with up and down key no longer working.

I think this can be fixable by adding some code to:

Context

We use getAnchoredPosition for primer-tooltip positioning (which is soon to be upstreamed to PVC).

In some scenarios, we observed the tooltip is not perfectly aligned at edge, particularly when there's no space for perfect edge alignment. Here is an example where align is end.

I was also able to recreate this issue in storybook:

Expected behavior

Since there’s no space to perfectly align end, I would have expected anchorAlignment to be adjusted to start instead. Is that not expected behavior?

For now, we're thinking of housing additional positioning logic in tooltip to address immediate issues, but this should probably be responsibility of getAnchoredPosition.

Copying behavior stories from https://github.com/primer/react/tree/main/src/stories

cc: @hectahertz

When the parent container of the floating and anchor element is a span that wraps (and has position: 'relative'), getAnchoredPosition breaks and returns a position that is way off.

In this example, the tooltip is attached to a label, and the parent of the labels is a span with position: relative. The labels wrap:

Once the behaviors are extracted, we should remove the duplicate code from primer/react and import them from here