fix: correct scrollable element check condition #7060

chirokas · 2024-09-20T15:14:00Z

Changed the logical operator from && to || because an element cannot be both document.documentElement and document.body at the same time.

✅ Pull Request Checklist:

Included link to corresponding React Spectrum GitHub Issue.
Added/updated unit tests and storybook for this change (for new code or code which already has tests).
Filled out test instructions.
Updated documentation (if it already exists for this component).
Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

* update illustrated message font size * remove v3 style props * remove unneccessary styling Co-authored-by: Yihui Liao <[email protected]>

* ActionMenu and positioning props for Menu

* Move Popover padding and add ref

* nextjs, macros, and components example app * package.json link path to rainbow was wrong * adding dropzone, taggroup, and illustratedmessage * using the npmjs version of unplogin-parcel-macros instead of linking * review feedback - config improvements Co-authored-by: Kyle Taborski <[email protected]>

* Avatar audit + fix to style macro to omit overriden default styles * removing avatars and isDisabled * audit button and buttongroup * more audit items * Adding component descriptions * Forgot to save * update with spectrum links and help text type update * properly only send DOM props to the Avatar * fix help text related types * update label type * whoops too much empty space * review comments

* I-P exports and descriptions

Menu has its own Divider so we can ensure style merge order (adobe#76)

* Use WF icons for InlineAlert * change prop name back * Get rid of cloneElement and use wrapper on context * swap to svgr and work on inline alert * fix sizing temporarily * Update all icon usages * inline styles, though i still think these are broken icons * review comments * update font colors of inline alert to match figma Co-authored-by: Daniel Lu <[email protected]>

* update type for Divider * update type for Dropzone * update type for Form * add JSDoc * update Divider JSDoc * Update Dropzone JSDoc * add more JSDocs * default Form size to M * formatting * add min height and width to divider based on orientation * switch defaults to medium values * update Divider large sizes * remove validationBehavior from Form * add PURE annotations Co-authored-by: Reid Barber <[email protected]> Co-authored-by: Rob Snow <[email protected]>

…ted, and other bug fixes (adobe#72) * adding ActionButton truncation and fixing Taggroup icon color when selected * fix icon only button sizing in ButtonGroup in safari not sure why the minWidth is being applied to non icon only buttons though... * Make help text not render anything if there isnt a error message to display when invalid * fix dropzone replace mesage banner positioning in Safari Co-authored-by: Rob Snow <[email protected]>

* update sizes to use auto instead of undefined * add none to theme * update to use none instead of auto * update style logic * Revert "add none to theme" This reverts commit 4f552e74de94475dd3809063b3a18ae548b3e740. * improve story * remove minWidth and minHeight Co-authored-by: Reid Barber <[email protected]>

* R-S audit * RadioGroup api audit and radio props from form context * renaming props and form props in radio * adding pure * updated comments and rest of files * fixing a variable usage Co-authored-by: Kyle Taborski <[email protected]>

Revert RAC Pending Button

* Yarn publish nightlies and verdaccio

* Fix nightly

Label breadcrumb collapse menu

* fix treeview stories controls * fix tree row padding if no checkbox and fix chromatic

* Fix icon builder dependencies

* replace Azure CLI with AzCopy * use wget * update command * remove azure-cli image * Revert "remove azure-cli image" This reverts commit 07b7ed0.

* fix path in verdaccio deploy * build on PR * revert * enable comments on PR * loop over verdaccio dir * add back branch filters

) * Allow layout delegate to override shift selection behavior * Fix virtualizer unmounting and remounting when changing layout options * Automatically reduce card size based on available space * Fix TS strict --------- Co-authored-by: Daniel Lu <[email protected]>

…rget (adobe#7026) Co-authored-by: Daniel Lu <[email protected]>

* initialize segemented file * add some styling * add track styling and hcm * fix hcm * remove unused imports * export properly, add font weight * default select first item, fix hcm * remove trackStyle prop * remove more trackStyle stuff, update css * update stories * fix types, add jsdoc comments * update story again oops * fix typo * fix types * remove space * add translation * update internal context prop name, add transition * remove transition, fix keyboard in rtl * update string formatter * initialize animations (it's very janky) * require aria label, remove translation * fix lint * fix ts * update props * fix spacing * add aria label to stories * update animation using flip, fix selectedValue when radio group is disabled * keep speed the same * fix lint and small other fixes * add icon only spacing * update segmented control item prop name * fix lint * small fixes * fix alignment with text and icons, fix wiggling for ltr * fix wiggling in rtl languages * update icon only buttons * revert useRadioGroupState change * revert changes to registration * fix lint --------- Co-authored-by: Robert Snow <[email protected]>

RAC and S2 Pending Button

Co-authored-by: Abel John <[email protected]> Co-authored-by: Robert Snow <[email protected]> Co-authored-by: Daniel Lu <[email protected]>

Co-authored-by: Daniel Lu <[email protected]>

Co-authored-by: Teemu Andersèn <[email protected]> Co-authored-by: Robert Snow <[email protected]> Co-authored-by: Reid Barber <[email protected]> Co-authored-by: Daniel Lu <[email protected]>

feat: forward styles to VirtualizerItem

snowystinger · 2024-09-24T03:05:03Z

Was anything breaking with this? It'd be good to be able to verify the behavior update. This is a line from 4 years ago, so curious how you found it.

chirokas · 2024-09-27T15:10:30Z

What I mean is that this could be a typo, and it should use the || operator to check conditions, just like in line 127. Also, from lines 127 to 130, it’s clear that in the onTouchStart event, if scrollable is either document.documentElement or document.body, there’s no need to handle with overscroll-behavior, since in the onTouchMove event, it will prevent the window from scrolling. If I got it wrong and it affected your work, I’m sorry about that.

snowystinger · 2024-09-29T22:26:38Z

No no, you misunderstand. I haven't seen it adversely affect anything. I'm curious how you noticed it.

chirokas · 2024-09-30T07:08:20Z

I’ve always been a fan of the React Spectrum library, so I like to dig into its source code whenever I have some free time. Recently, while debugging, I noticed that a specific if condition (on line 113) wasn’t being triggered, which got me curious. I started looking into it more, but now that I think about it, I might’ve messed something up in the process. I’m really sorry about that.

chirokas · 2024-09-30T07:53:49Z

I still don’t get what scrollable === document.documentElement && scrollable === document.body (document.documentElement === document.body ?) means in the code. Can you explain it simply?

devongovett and others added 30 commits March 6, 2024 10:14

Add css prop and unsafe style support (adobe#52)

e754213

Use default cursor for help text on disabled elements (adobe#55)

5678af7

Add LinkButton component (adobe#58)

bd89829

Fix missing type in module (adobe#59)

b5fcffa

Theme audit (adobe#63)

9561f2f

Remove V3 StyleProps from IllustratedMessage (adobe#57)

f9276d2

* update illustrated message font size * remove v3 style props * remove unneccessary styling Co-authored-by: Yihui Liao <[email protected]>

ActionMenu and positioning props for Menu (adobe#61)

ebdf8f9

* ActionMenu and positioning props for Menu

Static backgrounds that don't interfere with state (adobe#56)

ba5341d

Move Popover padding and add ref (adobe#60)

5c3ec65

* Move Popover padding and add ref

setup chromatic (adobe#65)

1c63b44

Audit T components (adobe#68)

0c86c7e

fix outline button disabled and keyboard focus styles (adobe#69)

b4cc790

I-P Audit (adobe#70)

aaa295c

I-P exports and descriptions (adobe#74)

a2cedc3

* I-P exports and descriptions

Menu has its own Divider so we can ensure style merge order (adobe#76)

c480260

Menu has its own Divider so we can ensure style merge order (adobe#76)

Menu placement fixes (adobe#71)

c156532

Icon fixes from testing (adobe#79)

4eaf766

Icons move fallback color out of page (adobe#82)

2db28b9

R-S audit (adobe#75)

3fb81a6

* R-S audit * RadioGroup api audit and radio props from form context * renaming props and form props in radio * adding pure * updated comments and rest of files * fixing a variable usage Co-authored-by: Kyle Taborski <[email protected]>

Remove border radius for fullscreen takeover dialogs (adobe#78)

1e65bb9

Audit Menu/Dialog/StatusLight Components (adobe#80)

93c00c5

Dialog: Fixing header and close button position (adobe#81)

939f8fd

Setup build to publish package (adobe#73)

a4f7b8c

Build icons with parcel (adobe#85)

ccd8619

snowystinger and others added 23 commits September 6, 2024 07:36

Revert RAC Pending Button (adobe#6986)

10c31a7

Revert RAC Pending Button

Yarn publish nightlies and verdaccio (adobe#6975)

c5a45b2

* Yarn publish nightlies and verdaccio

Fix nightly (adobe#7001)

641446f

* Fix nightly

Label breadcrumb collapse menu (adobe#6979)

17ad099

Label breadcrumb collapse menu

Fix useId updater overwrite (adobe#6788)

4980928

Fix Tree row padding when chevron isn't present (adobe#6545)

db51509

* fix treeview stories controls * fix tree row padding if no checkbox and fix chromatic

Fix icon builder dependencies (adobe#7018)

086ad31

* Fix icon builder dependencies

S2 CardView and ghost loading (adobe#6978)

8ab4f20

Use content hashes in CSS modules (adobe#7029)

437b381

CI: Improve build times with azcopy (adobe#7031)

7341499

* replace Azure CLI with AzCopy * use wget * update command * remove azure-cli image * Revert "remove azure-cli image" This reverts commit 07b7ed0.

CI: fix verdaccio deploy step (adobe#7035)

9da4651

* fix path in verdaccio deploy * build on PR * revert * enable comments on PR * loop over verdaccio dir * add back branch filters

Workaround Android issue where click events are fired on the wrong ta…

c904e06

…rget (adobe#7026) Co-authored-by: Daniel Lu <[email protected]>

Implement S2 ProgressCircle with SVG (adobe#7036)

2ca5989

Fix CardView quiet heights in Safari (adobe#7046)

9e79420

S2 Card updates (adobe#7053)

2bd6e20

RAC and S2 Pending Button (adobe#7002)

988b4d4

RAC and S2 Pending Button

fix conditional useHref call (adobe#7032)

097c8fc

Co-authored-by: Abel John <[email protected]> Co-authored-by: Robert Snow <[email protected]> Co-authored-by: Daniel Lu <[email protected]>

fix(internationalized/date): missing endOfWeek overload (adobe#6749)

b2e8ee6

Co-authored-by: Daniel Lu <[email protected]>

Fix useDateSegment null ref error adobe#6988 (adobe#6989)

53413af

Co-authored-by: Teemu Andersèn <[email protected]> Co-authored-by: Robert Snow <[email protected]> Co-authored-by: Reid Barber <[email protected]> Co-authored-by: Daniel Lu <[email protected]>

feat: forward styles VirtualizerItem (adobe#6998)

6193c40

feat: forward styles to VirtualizerItem

fix: correct scrollable element check condition

2bd826b

devongovett force-pushed the main branch from fc5c8bd to e60fb42 Compare September 28, 2024 00:55

devongovett force-pushed the main branch from a79adcf to 3013156 Compare September 30, 2024 21:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: correct scrollable element check condition #7060

fix: correct scrollable element check condition #7060

chirokas commented Sep 20, 2024

snowystinger commented Sep 24, 2024

chirokas commented Sep 27, 2024

snowystinger commented Sep 29, 2024

chirokas commented Sep 30, 2024

chirokas commented Sep 30, 2024

fix: correct scrollable element check condition #7060

Are you sure you want to change the base?

fix: correct scrollable element check condition #7060

Conversation

chirokas commented Sep 20, 2024

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Project:

snowystinger commented Sep 24, 2024

chirokas commented Sep 27, 2024

snowystinger commented Sep 29, 2024

chirokas commented Sep 30, 2024

chirokas commented Sep 30, 2024