Shared behaviors for JavaScript components
npm install @primer/behaviors
or
yarn add @primer/behaviors
Shared behaviors for JavaScript components
License: MIT License
Shared behaviors for JavaScript components
npm install @primer/behaviors
or
yarn add @primer/behaviors
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>
)
}
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):
Additional context
Add any other context about the problem here.
related - primer/react#2454
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
start a project with vite/vitest
render an AutoComplete in a test using the jsdom environment
see jsdom throw
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"
}
No response
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.
w/ help from @colebemis
Copying behavior stories from https://github.com/primer/react/tree/main/src/stories
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:
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
.
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:
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:
Line 261 in 92e06b5
Ignore please!
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:
Line 585 in f799310
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.
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
Once the behaviors are extracted, we should remove the duplicate code from primer/react and import them from here
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
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 |
---|---|
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. :)
Tooltip's position was set by
And the position was calculated from getAnchoredPosition
behaviors/src/anchored-position.ts
Line 140 in 6ed742f
In getAnchoredPosition
, the relativeRect
has the correct left
value for tooltip, if it was set to tooltip's left
style, it would align properly.
behaviors/src/anchored-position.ts
Lines 153 to 156 in 6ed742f
However, the relativeRect
was then passed to pureCalculateAnchoredPosition
and it returned a different left
.
behaviors/src/anchored-position.ts
Lines 290 to 379 in 6ed742f
Thank you for this awesome project!
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
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.