docs(split-view): added label for resizable examples for documentation #3800

Rajdeepc · 2023-11-14T14:34:09Z

Description

Added label property for resizable elements of split-view

Related issue(s)

[Bug][a11y]: Update resizable examples in documentation using the label prop available in the api #3383

Motivation and context

How has this been tested?

Test case 1
1. Go here
2. Do this
Test case 2
1. Go here
2. Do this

Screenshots (if appropriate)

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

Checklist

I have signed the Adobe Open Source CLA.
My code follows the code style of this project.
If my change required a change to the documentation, I have updated the documentation in this pull request.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.
I have reviewed at the Accessibility Practices for this feature, see: Aria Practices

Best practices

This repository uses conventional commit syntax for each commit message; note that the GitHub UI does not use this by default so be cautious when accepting suggested changes. Avoid the "Update branch" button on the pull request and opt instead for rebasing your branch against main.

github-actions · 2023-11-14T14:36:44Z

Branch Preview

Visual regression test results

When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:

github-actions · 2023-11-14T14:41:06Z

Tachometer results

Chrome

action-menu permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	640 kB	188.39ms - 192.64ms	-	unsure 🔍 -2% - +2% -3.10ms - +3.07ms
branch	621 kB	188.29ms - 192.77ms	unsure 🔍 -2% - +2% -3.07ms - +3.10ms	-

menu permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	462 kB	252.97ms - 258.62ms	-	unsure 🔍 -1% - +2% -2.95ms - +4.19ms
branch	449 kB	252.98ms - 257.36ms	unsure 🔍 -2% - +1% -4.19ms - +2.95ms	-

picker permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	506 kB	651.85ms - 669.65ms	-	unsure 🔍 -0% - +3% -2.54ms - +18.39ms
branch	491 kB	647.33ms - 658.33ms	unsure 🔍 -3% - +0% -18.39ms - +2.54ms	-

sidenav permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	428 kB	343.15ms - 347.00ms	-	unsure 🔍 -1% - +1% -4.29ms - +2.01ms
branch	416 kB	343.72ms - 348.71ms	unsure 🔍 -1% - +1% -2.01ms - +4.29ms	-

split-button permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	712 kB	1853.54ms - 1856.57ms	-	unsure 🔍 -0% - +0% -0.76ms - +3.37ms
branch	696 kB	1852.36ms - 1855.16ms	unsure 🔍 -0% - +0% -3.37ms - +0.76ms	-

split-view permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	397 kB	37.34ms - 37.97ms	-	unsure 🔍 -4% - +0% -1.45ms - +0.11ms
branch	383 kB	37.61ms - 39.04ms	unsure 🔍 -0% - +4% -0.11ms - +1.45ms	-

switch permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	401 kB	27.55ms - 27.94ms	-	unsure 🔍 -0% - +1% -0.11ms - +0.35ms
branch	387 kB	27.51ms - 27.74ms	unsure 🔍 -1% - +0% -0.35ms - +0.11ms	-

Firefox

action-menu permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	640 kB	336.43ms - 345.09ms	-	unsure 🔍 -2% - +2% -7.30ms - +6.90ms
branch	621 kB	335.33ms - 346.59ms	unsure 🔍 -2% - +2% -6.90ms - +7.30ms	-

menu permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	462 kB	434.29ms - 445.03ms	-	unsure 🔍 -1% - +2% -6.20ms - +9.56ms
branch	449 kB	432.22ms - 443.74ms	unsure 🔍 -2% - +1% -9.56ms - +6.20ms	-

picker permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	506 kB	999.65ms - 1012.63ms	-	unsure 🔍 -1% - +1% -11.86ms - +12.22ms
branch	491 kB	995.82ms - 1016.10ms	unsure 🔍 -1% - +1% -12.22ms - +11.86ms	-

sidenav permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	428 kB	548.04ms - 557.40ms	-	unsure 🔍 -2% - +1% -12.93ms - +3.69ms
branch	416 kB	550.48ms - 564.20ms	unsure 🔍 -1% - +2% -3.69ms - +12.93ms	-

split-button permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	712 kB	1570.98ms - 1576.10ms	-	unsure 🔍 -0% - +0% -1.88ms - +3.96ms
branch	696 kB	1571.08ms - 1573.92ms	unsure 🔍 -0% - +0% -3.96ms - +1.88ms	-

split-view permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	397 kB	83.84ms - 88.52ms	-	unsure 🔍 -7% - +2% -6.26ms - +1.50ms
branch	383 kB	85.47ms - 91.65ms	unsure 🔍 -2% - +7% -1.50ms - +6.26ms	-

switch permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	401 kB	69.16ms - 73.52ms	-	unsure 🔍 -7% - +3% -4.92ms - +2.28ms
branch	387 kB	69.80ms - 75.52ms	unsure 🔍 -3% - +7% -2.28ms - +4.92ms	-

castastrophe · 2023-11-14T14:45:58Z

@Rajdeepc I added an idea to the original issue if you wanted to weigh in there!

najikahalsema

Could you actually give a value to the labels here?

Rajdeepc · 2023-11-15T07:10:05Z

Could you actually give a value to the labels here?

Adding a value on label will not change the aria-label in splitter since it is being hardcoded in the code
Here:

spectrum-web-components/packages/split-view/src/SplitView.ts

Line 186 in f0b2d73

const label =

May be we should make it more dynamic and let the users add a dynamic value to the label?

najikahalsema · 2023-11-15T18:47:19Z

Sorry, I accidentally closed the PR when trying to make this comment.

I think that the way the property is set though would allow for a custom label, though, right? Because the logic is

        const label =
            this.label || this.resizable ? 'Resize the panels' : undefined;

And the consumer can set this.label ... so are the only values 'Resize the panels' or undefined? What happens if you set this.label to some string value?

Rajdeepc · 2023-11-16T13:04:36Z

Sorry, I accidentally closed the PR when trying to make this comment.

I think that the way the property is set though would allow for a custom label, though, right? Because the logic is
        const label =
            this.label || this.resizable ? 'Resize the panels' : undefined;
And the consumer can set this.label ... so are the only values 'Resize the panels' or undefined? What happens if you set this.label to some string value?

@najikahalsema If we set this.label to some other string values. aria-label value still remains as 'Resize the panels` due to line 207

aria-label remains unchanged as this is expected since we are not adding this.label if we found this.label instead we are hardcoding.

najikahalsema · 2023-11-16T18:25:57Z

Okay. I gotcha. I guess we can merge like how it is now and make a follow up issue.

Westbrook

label without a value is a non-sensical addition here. We should include a value in this change.

We should address the issue that was created herein, and in particular the reality that I wrote the logic to apply the aria-label value incorrectly. 🙈

const label =
            this.label || this.resizable ? 'Resize the panels' : undefined;

Should likely be

const label =
            this.label || (this.resizable ? 'Resize the panels' : undefined);

Or possibly:

const label =
            this.resizable ? (this.label || 'Resize the panels') : undefined;

So that the label attribute actually does something here...

najikahalsema

Thanks, @Westbrook , for being able to articulate what I could not. ^^; Thanks for fixing this, Rajdeep.

…um-web-components into bug/label-splitview

#3800) * docs(split-view): added label for resizable examples for documentation * chore(split-view): updated readme and label logic --------- Co-authored-by: Rajdeep Chandra <rajdeepc@adobe.com> Co-authored-by: Najika Halsema Yoo <44980010+najikahalsema@users.noreply.github.com>

docs(split-view): added label for resizable examples for documentation

9f10bdf

Rajdeepc requested review from Westbrook and najikahalsema November 14, 2023 14:34

Rajdeepc added a11y Issues or PRs related to accessibility Component: Documentation Issues or PRs involving changes to docs or docs website. Component prefix is for Jira integration. Component: Split view labels Nov 14, 2023

Rajdeepc linked an issue Nov 14, 2023 that may be closed by this pull request

[Bug][a11y]: Update resizable examples in documentation using the label prop available in the api #3383

Closed

1 task

najikahalsema requested changes Nov 14, 2023

View reviewed changes

Merge branch 'main' into bug/label-splitview

5c85eae

najikahalsema approved these changes Nov 15, 2023 •

edited

Loading

View reviewed changes

najikahalsema self-requested a review November 15, 2023 18:45

najikahalsema closed this Nov 15, 2023

najikahalsema reopened this Nov 15, 2023

najikahalsema mentioned this pull request Nov 16, 2023

[split-view]: update label attribute to be customisable #3811

Closed

1 task

najikahalsema previously approved these changes Nov 16, 2023

View reviewed changes

Westbrook suggested changes Nov 20, 2023

View reviewed changes

Rajdeep Chandra added 2 commits November 30, 2023 17:03

Merge branch 'main' into bug/label-splitview

3a45e23

chore(split-view): updated readme and label logic

369fdcc

Rajdeepc dismissed najikahalsema’s stale review via 369fdcc November 30, 2023 11:41

Rajdeepc requested review from Westbrook and najikahalsema November 30, 2023 11:42

Rajdeepc self-assigned this Nov 30, 2023

najikahalsema approved these changes Nov 30, 2023

View reviewed changes

Merge branch 'main' into bug/label-splitview

3edc3eb

Westbrook approved these changes Nov 30, 2023

View reviewed changes

Rajdeep Chandra added 3 commits December 1, 2023 11:07

Merge branch 'main' into bug/label-splitview

d07c290

Merge branch 'bug/label-splitview' of https://github.com/adobe/spectr…

9818837

…um-web-components into bug/label-splitview

Merge branch 'main' into bug/label-splitview

94ff410

Westbrook merged commit 1ac5058 into main Dec 1, 2023

Westbrook deleted the bug/label-splitview branch December 1, 2023 20:58

docs(split-view): added label for resizable examples for documentation #3800

docs(split-view): added label for resizable examples for documentation #3800

Uh oh!

Conversation

Rajdeepc commented Nov 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issue(s)

Motivation and context

How has this been tested?

Screenshots (if appropriate)

Types of changes

Checklist

Best practices

Uh oh!

github-actions bot commented Nov 14, 2023

Branch Preview

Uh oh!

github-actions bot commented Nov 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tachometer results

action-menu permalink

menu permalink

picker permalink

sidenav permalink

split-button permalink

split-view permalink

switch permalink

action-menu permalink

menu permalink

picker permalink

sidenav permalink

split-button permalink

split-view permalink

switch permalink

Uh oh!

castastrophe commented Nov 14, 2023

Uh oh!

najikahalsema left a comment

Choose a reason for hiding this comment

Uh oh!

Rajdeepc commented Nov 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

najikahalsema commented Nov 15, 2023

Uh oh!

Rajdeepc commented Nov 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

najikahalsema commented Nov 16, 2023

Uh oh!

Westbrook left a comment

Choose a reason for hiding this comment

Uh oh!

najikahalsema left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Rajdeepc commented Nov 14, 2023 •

edited

Loading

github-actions bot commented Nov 14, 2023 •

edited

Loading

Rajdeepc commented Nov 15, 2023 •

edited

Loading

Rajdeepc commented Nov 16, 2023 •

edited

Loading