Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

On hold - V3 splitview followup #37

Closed
wants to merge 21 commits into from
Closed

Conversation

snowystinger
Copy link
Member

Closes https://jira.corp.adobe.com/browse/RSP-1183, https://jira.corp.adobe.com/browse/RSP-1185, https://jira.corp.adobe.com/browse/RSP-1186, https://jira.corp.adobe.com/browse/RSP-1184, https://jira.corp.adobe.com/browse/RSP-1190, https://jira.corp.adobe.com/browse/RSP-1188, https://jira.corp.adobe.com/browse/RSP-1187
and there seem to be two follow up stories
so this also closes the subtask in the other one https://jira.corp.adobe.com/browse/RSP-1108

✅ Pull Request Checklist:

  • Included link to corresponding 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 exist for this component).
  • Looked at the Accessibility Standard and/or talked to @mijordan about Accessibility for this feature.

📝 Test Instructions:

Should match https://www.w3.org/TR/wai-aria-practices-1.1/#windowsplitter
clarified some language in the spec vs what is implemented in v2

Rob Snow 2:22 PM
hey! question on aria specs
https://www.w3.org/TR/wai-aria-practices-1.1/#windowsplitter
this states
The separator element has the aria-valuenow property set to a decimal value representing the current position of the separator.
The separator element has the aria-valuemin property set to a decimal value that represents the position where the primary pane has its minimum size. This is typically 0.
The separator element has the aria-valuemax property set to a decimal value that represents the position where the primary pane has its maximum size . This is typically 100.
how should the values change between collapsible and not?
for instance, if min size is 200 and max is 1000 and we're not collapsible, i'm assuming this for aria
aria min 0 at size 200, aria max 100 at 1000 and valuenow, a percent between the two
for collapsible though, i'm assuming
aria min 0 at size 0? or is 0 still at size 200?

w3.orgw3.org
WAI-ARIA Authoring Practices 1.1
This document provides readers with an understanding of how to use WAI-ARIA 1.1 [WAI-ARIA] to create accessible rich internet applications. It describes considerations that might not be evident to most authors from the WAI-ARIA specification alone and recommends approaches to make widgets, navigation, and behaviors accessible using WAI-ARIA roles, states, and properties. This document is directed primarily to Web application developers, but the guidance is also useful for user agent and assistive technology developers.

James Nurthen 2:24 PM
In this case I think I’d have aria-min as 20 and aria-max as 100
having a min of 0 would give the impression that it can be collapsed to 0
… but realistically the screen reader will probably just read a value as a percentage between the min and the max and not read the actual values of min and max at all
James Nurthen 2:27 PM
so in my example it would read 0% when it gets to the 200 value and 100% at 1000

🧢 Your Team:

RSP

…home button for non-collapsible to make it reflect aria standards

Change useDrag1D to be useMoveable and fire off events in 2D
Use axis to determine reversals because we may only want to flip over one axis at a time
…ction that is passed the previous state value

Write tests for useControlledState to verify all of this
Change SplitView to rely more fully on useControlledState
Add actions to controlled SplitView stories so that behavior can be confirmed there too
…d to what I thought it would be, will ask about in comments
# Conflicts:
#	packages/@react-aria/interactions/package.json
#	packages/@react-aria/interactions/src/index.ts
# Conflicts:
#	packages/@react-aria/interactions/package.json
#	packages/@react-aria/splitview/package.json
#	packages/@react-aria/utils/src/index.ts
#	packages/@react-spectrum/splitview/package.json
#	packages/@react-spectrum/splitview/src/SplitView.tsx
#	packages/@react-stately/splitview/src/useSplitViewState.ts
#	packages/@react-stately/utils/src/useControlledState.ts
#	yarn.lock
…into v3-splitview-followup

# Conflicts:
#	packages/@react-aria/i18n/package.json
#	packages/@react-aria/splitview/package.json
#	packages/@react-aria/splitview/src/useSplitView.ts
#	packages/@react-aria/utils/src/index.ts
#	packages/@react-aria/utils/src/useDrag1D.ts
#	packages/@react-spectrum/splitview/package.json
#	packages/@react-spectrum/splitview/test/SplitView.test.js
#	packages/@react-stately/splitview/package.json
#	packages/@react-types/shared/src/index.d.ts
#	packages/@react-types/shared/src/splitview.d.ts
@github-actions
Copy link

Build successful! View the storybook

@LFDanLu
Copy link
Member

LFDanLu commented Nov 5, 2019

@snowystinger looks like you got a conflict. Seems pretty straight forward to resolve, gona resolve it locally and keep reviewing

Copy link
Member

@ktabors ktabors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When testing in storybook I found a bug in vertical where I click on the adjuster, I try to move down and it jumps up. I try to resize and it is confined to the new size of where it jumped to. Do this a few times, until it is almost at the top and not it jumps down.

What does the scrolling content story do?

@@ -1,3 +1,4 @@
import {direction} from '@react-types/shared';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add this to the package.json

// relative position of cursor to a container
// or as keyboard events

export function useMoveable({containerRef, flipAxis, onHover, onDrag, onPositionChange, onIncrement, onDecrement, onIncrementToMax, onDecrementToMin, onCollapseToggle}: UseMoveableProps): AllHTMLAttributes<HTMLElement> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't you deconstruct the props as below?

# Conflicts:
#	packages/@react-stately/utils/src/useControlledState.ts
@adobe-bot
Copy link

Build successful! View the storybook

Merge branch 'master' into v3-splitview-followup

# Conflicts:
#	packages/@react-aria/splitview/package.json
#	packages/@react-aria/splitview/src/useSplitView.ts
#	packages/@react-spectrum/splitview/package.json
#	packages/@react-spectrum/splitview/src/SplitView.tsx
#	packages/@react-spectrum/splitview/stories/SplitView.stories.js
#	packages/@react-spectrum/splitview/test/SplitView.test.js
#	packages/@react-stately/utils/src/useControlledState.ts
#	packages/@react-stately/utils/test/useControlledState.test.js
#	packages/@react-types/shared/src/index.d.ts
#	packages/@react-types/shared/src/splitview.d.ts
@adobe-bot
Copy link

Build successful! View the storybook

…view-followup

# Conflicts:
#	packages/@react-aria/i18n/src/context.tsx
#	packages/@react-aria/splitview/package.json
#	packages/@react-aria/splitview/src/useSplitView.ts
#	packages/@react-aria/utils/src/getOffset.ts
#	packages/@react-aria/utils/src/useDrag1D.ts
#	packages/@react-spectrum/splitview/package.json
#	packages/@react-stately/splitview/package.json
#	packages/@react-stately/splitview/src/useSplitViewState.ts
#	packages/@react-types/shared/src/index.d.ts
#	packages/@react-types/shared/src/splitview.d.ts
@adobe-bot
Copy link

Build successful! 🎉

@dannify dannify changed the title V3 splitview followup On hold - V3 splitview followup Jul 12, 2020
@dannify dannify changed the base branch from master to main July 12, 2020 00:53
@snowystinger
Copy link
Member Author

Closing since useMove work has made a lot of this obsolete. When we work on splitview again, we can look here for some of the remaining work

@snowystinger snowystinger deleted the v3-splitview-followup branch December 6, 2021 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants