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

feat(action-menu): added static attribute support #3573

Merged
merged 14 commits into from
Aug 30, 2023

Conversation

blunteshwar
Copy link
Collaborator

Description

Added static attribute support for action-menu

Related issue(s)

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.

@blunteshwar blunteshwar linked an issue Aug 23, 2023 that may be closed by this pull request
1 task
@github-actions
Copy link

github-actions bot commented Aug 23, 2023

Tachometer results

Chrome

action-bar permalink

Version Bytes Avg Time vs remote vs branch
npm latest 477 kB 124.33ms - 126.07ms - unsure 🔍
-0% - +2%
-0.13ms - +2.06ms
branch 472 kB 123.57ms - 124.90ms unsure 🔍
-2% - +0%
-2.06ms - +0.13ms
-

action-menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 636 kB 249.49ms - 252.90ms - slower ❌
0% - 2%
0.71ms - 5.52ms
branch 631 kB 246.38ms - 249.78ms faster ✔
0% - 2%
0.71ms - 5.52ms
-

menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 450 kB 371.41ms - 375.52ms - slower ❌
1% - 3%
3.80ms - 9.84ms
branch 445 kB 364.43ms - 368.86ms faster ✔
1% - 3%
3.80ms - 9.84ms
-

overlay permalink

Version Bytes Avg Time vs remote vs branch
npm latest 488 kB 98.83ms - 102.80ms - unsure 🔍
-1% - +3%
-1.49ms - +2.72ms
branch 472 kB 99.50ms - 100.90ms unsure 🔍
-3% - +1%
-2.72ms - +1.49ms
-

picker permalink

Version Bytes Avg Time vs remote vs branch
npm latest 491 kB 931.26ms - 954.26ms - slower ❌
2% - 6%
21.39ms - 52.46ms
branch 486 kB 895.39ms - 916.28ms faster ✔
2% - 6%
21.39ms - 52.46ms
-

popover permalink

Version Bytes Avg Time vs remote vs branch
npm latest 372 kB 36.86ms - 37.20ms - unsure 🔍
-1% - +1%
-0.49ms - +0.27ms
branch 367 kB 36.80ms - 37.48ms unsure 🔍
-1% - +1%
-0.27ms - +0.49ms
-

split-button permalink

Version Bytes Avg Time vs remote vs branch
npm latest 694 kB 1893.28ms - 1896.74ms - unsure 🔍
-0% - +0%
-1.60ms - +3.64ms
branch 689 kB 1892.02ms - 1895.96ms unsure 🔍
-0% - +0%
-3.64ms - +1.60ms
-

tooltip permalink

Version Bytes Avg Time vs remote vs branch
npm latest 554 kB 92.06ms - 93.94ms - unsure 🔍
-1% - +1%
-0.94ms - +1.37ms
branch 549 kB 92.11ms - 93.46ms unsure 🔍
-1% - +1%
-1.37ms - +0.94ms
-
Firefox

action-bar permalink

Version Bytes Avg Time vs remote vs branch
npm latest 477 kB 345.10ms - 374.50ms - unsure 🔍
-4% - +6%
-15.18ms - +20.86ms
branch 472 kB 346.53ms - 367.39ms unsure 🔍
-6% - +4%
-20.86ms - +15.18ms
-

action-menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 636 kB 475.22ms - 513.18ms - unsure 🔍
-3% - +7%
-13.39ms - +34.39ms
branch 631 kB 469.18ms - 498.22ms unsure 🔍
-7% - +3%
-34.39ms - +13.39ms
-

menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 450 kB 709.33ms - 736.31ms - unsure 🔍
-3% - +4%
-20.59ms - +28.55ms
branch 445 kB 698.30ms - 739.38ms unsure 🔍
-4% - +3%
-28.55ms - +20.59ms
-

overlay permalink

Version Bytes Avg Time vs remote vs branch
npm latest 477 kB 238.51ms - 261.01ms - unsure 🔍
-7% - +7%
-17.54ms - +16.58ms
branch 486 kB 237.41ms - 263.07ms unsure 🔍
-7% - +7%
-16.58ms - +17.54ms
-

picker permalink

Version Bytes Avg Time vs remote vs branch
npm latest 491 kB 1287.29ms - 1336.67ms - slower ❌
0% - 6%
1.23ms - 70.85ms
branch 486 kB 1251.40ms - 1300.48ms faster ✔
0% - 5%
1.23ms - 70.85ms
-

popover permalink

Version Bytes Avg Time vs remote vs branch
npm latest 372 kB 122.28ms - 139.32ms - unsure 🔍
-8% - +12%
-10.58ms - +14.82ms
branch 367 kB 119.26ms - 138.10ms unsure 🔍
-11% - +8%
-14.82ms - +10.58ms
-

split-button permalink

Version Bytes Avg Time vs remote vs branch
npm latest 694 kB 1622.69ms - 1635.27ms - unsure 🔍
-1% - +1%
-9.66ms - +11.18ms
branch 689 kB 1619.91ms - 1636.53ms unsure 🔍
-1% - +1%
-11.18ms - +9.66ms
-

tooltip permalink

Version Bytes Avg Time vs remote vs branch
npm latest 554 kB 210.13ms - 239.51ms - unsure 🔍
-5% - +12%
-9.70ms - +25.50ms
branch 549 kB 207.23ms - 226.61ms unsure 🔍
-11% - +4%
-25.50ms - +9.70ms
-

packages/action-menu/src/ActionMenu.ts Show resolved Hide resolved
packages/action-menu/src/ActionMenu.ts Outdated Show resolved Hide resolved
packages/action-menu/stories/action-menu.stories.ts Outdated Show resolved Hide resolved
packages/action-menu/stories/action-menu.stories.ts Outdated Show resolved Hide resolved
@Rajdeepc Rajdeepc self-requested a review August 23, 2023 07:03
Rajdeepc
Rajdeepc previously approved these changes Aug 23, 2023
Copy link
Contributor

@Rajdeepc Rajdeepc left a comment

Choose a reason for hiding this comment

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

LGTM!!

@Rajdeepc Rajdeepc added a11y Issues related to accessibility Component: Action Menu labels Aug 29, 2023
Copy link
Contributor

@Westbrook Westbrook left a comment

Choose a reason for hiding this comment

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

Sorry, one last thing!

Catch this up to main and decorate more selectively and let's land this baby. 🛬


import '@spectrum-web-components/icons-workflow/icons/sp-icon-settings.js';
import type { MenuItem } from '@spectrum-web-components/menu/src/MenuItem.js';

export default {
component: 'sp-action-menu',
title: 'Action menu',
decorators: [makeOverBackground()],
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
decorators: [makeOverBackground()],

@Rajdeepc Rajdeepc requested a review from Westbrook August 29, 2023 14:46
@Rajdeepc
Copy link
Contributor

Sorry, one last thing!

Catch this up to main and decorate more selectively and let's land this baby. 🛬

Changes updated. Let me know if you see any more issues here.

Copy link
Contributor

@Westbrook Westbrook left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for working through the Storybook delivery, makes understanding this addition a whole lot easier!

@Westbrook Westbrook merged commit 25889a8 into main Aug 30, 2023
@Westbrook Westbrook deleted the blunteshwar/static-support-action-menu branch August 30, 2023 11:48
blunteshwar added a commit that referenced this pull request Sep 12, 2023
* fix(action-menu): added static attribute support

* fix(action-menu): replaced 'undefined' with 'none'

* fix(action-menu): updated the tests

* fix(action-menu): added golden hash key

* fix(action-menu): added labels for control

* fix: storybook none value maps to undefined

* fix: storybook decorators reference for individual stories

* chore: updated golden image cache

---------

Co-authored-by: Rajdeep Chandra <rajdeepc@adobe.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y Issues related to accessibility Component: Action Menu
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat]: add static attribute support to Action Menu
3 participants