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.

Sign up for GitHub

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

Jump to bottom

refactor!: update menu-bar to use vaadin-menu-bar-item #5339

Merged
merged 4 commits into from
Jan 24, 2023
Merged

refactor!: update menu-bar to use vaadin-menu-bar-item #5339

merged 4 commits into from
Jan 24, 2023

Conversation

ugur-vaadin
Copy link
Contributor

@ugur-vaadin ugur-vaadin commented Jan 19, 2023 •
edited by web-padawan
Loading

Description

Fixes #3010

Based on #5353 and _tagNamePrefix API added in that PR.

Depends on #5354 which includes changes for the list-box.

Type of change

  • Refactor

@ugur-vaadin ugur-vaadin linked an issue Jan 19, 2023 that may be closed by this pull request
Update menu-bar to use vaadin-menu-bar-item component for sub-menus #3010
Closed
@ugur-vaadin ugur-vaadin marked this pull request as ready for review January 22, 2023 21:36
web-padawan
web-padawan requested changes Jan 23, 2023
View reviewed changes
Copy link
Member

@web-padawan web-padawan left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. To me it feels like this change needs a bit more work, to also update vaadin-context-menu-overlay and vaadin-context-menu-list-box tag names.

I'll continue from here, will probably create a few smaller PRs before updating this one.

Screenshot 2023-01-23 at 10 07 37

@web-padawan web-padawan mentioned this pull request Jan 23, 2023
Merged
@web-padawan web-padawan force-pushed the 3010-update-menu-bar-to-use-vaadin-menu-bar-item-component-for-sub-menus branch from 80e5343 to 388f058 Compare January 23, 2023 10:47
@web-padawan web-padawan marked this pull request as draft January 23, 2023 10:48
@web-padawan web-padawan changed the title refactor!: introduce base class for menu items refactor!: update menu-bar to use vaadin-menu-bar-item Jan 23, 2023
@web-padawan web-padawan mentioned this pull request Jan 23, 2023
Closed
@web-padawan web-padawan force-pushed the 3010-update-menu-bar-to-use-vaadin-menu-bar-item-component-for-sub-menus branch 2 times, most recently from 0316a09 to ea9cdd5 Compare January 23, 2023 14:46
@web-padawan web-padawan force-pushed the 3010-update-menu-bar-to-use-vaadin-menu-bar-item-component-for-sub-menus branch from ea9cdd5 to 0a8cd8a Compare January 23, 2023 14:47
@web-padawan web-padawan marked this pull request as ready for review January 23, 2023 14:48
@web-padawan web-padawan requested a review from vursen January 23, 2023 14:48
@web-padawan web-padawan force-pushed the 3010-update-menu-bar-to-use-vaadin-menu-bar-item-component-for-sub-menus branch from 0a8cd8a to 77607ec Compare January 23, 2023 16:03
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@web-padawan web-padawan removed the request for review from tomivirkki January 24, 2023 08:03
web-padawan
web-padawan approved these changes Jan 24, 2023
View reviewed changes
Copy link
Member

@web-padawan web-padawan left a comment

Choose a reason for hiding this comment

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

LGTM to un-block the PR even though it's actually mostly my code now 🙈

@web-padawan web-padawan merged commit 3800700 into master Jan 24, 2023
@web-padawan web-padawan deleted the 3010-update-menu-bar-to-use-vaadin-menu-bar-item-component-for-sub-menus branch January 24, 2023 08:07
@web-padawan web-padawan mentioned this pull request Jan 27, 2023
Merged
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.0.0.alpha10 and is also targeting the upcoming stable 24.0.0 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vursen vursen vursen approved these changes

@web-padawan web-padawan web-padawan approved these changes

Assignees
No one assigned
Labels
prerelease version for Vaadin 24.0.0 Released with Vaadin 24.0.0.alpha10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update menu-bar to use vaadin-menu-bar-item component for sub-menus
4 participants
@ugur-vaadin @vaadin-bot @vursen @web-padawan