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

feat(NcUserBubble): add RouterLink support #5708

Merged
merged 1 commit into from
Aug 5, 2024
+15 −1
Merged

feat(NcUserBubble): add RouterLink support #5708

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/components/NcUserBubble/NcUserBubble.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export default {
<component :is="isLinkComponent"
class="user-bubble__content"
:style="styles.content"
:to="to"
:href="hasUrl ? url : null"
:class="{ 'user-bubble__content--primary': primary }"
v-bind="attrs"
Expand Down Expand Up @@ -108,6 +109,7 @@ import NcUserBubbleDiv from './NcUserBubbleDiv.vue'
import NcAvatar from '../NcAvatar/index.js'
import NcPopover from '../NcPopover/index.js'
import Vue from 'vue'
import { RouterLink } from 'vue-router'

export default {
name: 'NcUserBubble',
Expand Down Expand Up @@ -160,6 +162,13 @@ export default {
}
},
},
/**
* Use bubble as a router-link for in-app navigation
*/
to: {
type: [String, Object],
default: undefined,
},
/**
* Default popover state. Requires the UserBubble
* to have some content to render inside the popover
Expand Down Expand Up @@ -240,7 +249,9 @@ export default {
},

isLinkComponent() {
Antreesy marked this conversation as resolved.
Show resolved Hide resolved
return this.hasUrl ? 'a' : 'div'
return this.hasUrl
? (this.to ? RouterLink : 'a')
: 'div'
Copy link
Contributor Author

@Antreesy Antreesy Aug 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Resolved your suggestions, thanks!
I kept only RouterLink support, and cursor styling for anchors.

Can we address this in follow-up, maybe? or <a role="button"> would suffice?

Suggested change
: 'div'
: (this.popoverEmpty ? 'div' : 'button')

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, let's do it in a follow up. It is not a part of the original issue. TBH I even don't remember, where we use use bubble with popover...

Antreesy marked this conversation as resolved.
Show resolved Hide resolved
},

popoverEmpty() {
Expand Down Expand Up @@ -329,4 +340,7 @@ export default {
}
}

a.user-bubble__content {
Antreesy marked this conversation as resolved.
Show resolved Hide resolved
cursor: pointer;
}
</style>
Loading