Skip to content
This repository has been archived by the owner on Jan 3, 2024. It is now read-only.

Add breadcrumb contextmenu #1786

Merged
merged 6 commits into from
Nov 24, 2021
Merged

Add breadcrumb contextmenu #1786

merged 6 commits into from
Nov 24, 2021

Conversation

pascalwengerter
Copy link
Contributor

@pascalwengerter pascalwengerter commented Nov 17, 2021 •
edited by JammingBen
Loading

Related Issue

How Has This Been Tested?

  • Manually

Types of changes

  • New feature (non-breaking change which adds functionality)
  • Tests

Checklist:

  • Code changes
  • Unit tests added/updated
  • Documentation updated

@pascalwengerter pascalwengerter mentioned this pull request Nov 17, 2021
Closed
pascalwengerter
pascalwengerter commented Nov 19, 2021
View reviewed changes
Copy link
Contributor Author

@pascalwengerter pascalwengerter left a comment

Choose a reason for hiding this comment

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

Needs tests for the next prop & slot if this approach works in web

@JammingBen JammingBen mentioned this pull request Nov 22, 2021
Merged
9 tasks
@pascalwengerter
Copy link
Contributor Author

@janackermann @JammingBen feel free to update/review/merge this one if it suits your needs in web!

@JammingBen
Copy link
Contributor

@pascalwengerter It's working just fine 👍 Some tests here would be nice though, what do you think?

@pascalwengerter
Copy link
Contributor Author

@pascalwengerter It's working just fine +1 Some tests here would be nice though, what do you think?

Yes please. Checking that the slot doesn't get displayed if the value is set to false or if it is set to true but only one breadcrumb item is handed over (or sth like that) 👌🏽

@JammingBen
Copy link
Contributor

@pascalwengerter I updated the PR with unit tests and added showContextmenu as computed property. Please have a look, since you probably can't review your own PR.

@pascalwengerter
Copy link
Contributor Author

@pascalwengerter I updated the PR with unit tests and added showContextmenu as computed property. Please have a look, since you probably can't review your own PR.

Can't approve but LGTM ;)

@kulmann kulmann changed the title Add PoC for breadcrumb contextmenu Add breadcrumb contextmenu Nov 24, 2021
kulmann
kulmann suggested changes Nov 24, 2021
View reviewed changes
src/components/molecules/OcBreadcrumb/OcBreadcrumb.vue Outdated Show resolved Hide resolved
src/components/molecules/OcBreadcrumb/OcBreadcrumb.vue Outdated Show resolved Hide resolved
src/components/molecules/OcBreadcrumb/OcBreadcrumb.spec.js Outdated Show resolved Hide resolved
src/components/molecules/OcBreadcrumb/OcBreadcrumb.spec.js Outdated Show resolved Hide resolved
src/components/molecules/OcBreadcrumb/OcBreadcrumb.spec.js Outdated Show resolved Hide resolved
src/components/molecules/OcBreadcrumb/OcBreadcrumb.spec.js Outdated Show resolved Hide resolved
src/components/molecules/OcBreadcrumb/OcBreadcrumb.spec.js Outdated Show resolved Hide resolved
src/components/molecules/OcBreadcrumb/OcBreadcrumb.spec.js Outdated Show resolved Hide resolved
@JammingBen JammingBen requested a review from kulmann November 24, 2021 09:04
@JammingBen JammingBen requested a review from kulmann November 24, 2021 12:33
@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

100.0% 100.0% Coverage
0.0% 0.0% Duplication

kulmann
kulmann approved these changes Nov 24, 2021
View reviewed changes
Copy link
Contributor

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

Awesome! 😍

@kulmann kulmann merged commit c3d8cc6 into master Nov 24, 2021
@delete-merged-branch delete-merged-branch bot deleted the 17112021_last-breadcrumb-contextmenu branch November 24, 2021 12:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kulmann kulmann kulmann approved these changes

@JammingBen JammingBen Awaiting requested review from JammingBen

@AlexAndBear AlexAndBear Awaiting requested review from AlexAndBear

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pascalwengerter @JammingBen @kulmann