Skip to content

Commit 56198e0

Browse files
authored
Preserve URL hash when clicking 'Mark all as read' (#17348)
* Preserve URL hash when clicking 'Mark all as read' The anchor used href="#" and did not prevent the default click event, so clicking it navigated the browser to '#' and wiped any existing URL hash fragment (e.g. #pod). This also broke extensions using LocationConfig's hash field to scope injected columns/components to a specific fragment. Fixes #16923 * Use RcButton (variant=link) for 'Mark all as read' action Replaces the <a href="#"> with <RcButton variant="link" size="small">. A button is semantically correct for this action and removes the default anchor navigation that was stripping the URL hash fragment on click. Addresses reviewer feedback on PR #17348. * Match original anchor appearance for 'Mark all as read' Switch the RcButton to the ghost variant and scope a small style override so the rendered button has identical metrics to the previous <a href="#">: inherited font-size/line-height, no padding or min-height, link color, and the global anchor hover behavior (body-text color + underline). No visible change from the original UI. * Drop unused CSS overrides on 'Mark all as read' gap only affects flex children and the slot has a single text node. The ._hover selector is RcButton's programmatic hover class, which nothing toggles here.
1 parent c1d52a2 commit 56198e0

2 files changed

Lines changed: 100 additions & 8 deletions

File tree

shell/components/nav/NotificationCenter/NotificationHeader.vue

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,19 @@
22
import { useStore } from 'vuex';
33
import { computed, inject, ref } from 'vue';
44
import { DropdownContext, defaultContext } from '@components/RcDropdown/types';
5+
import RcButton from '@components/RcButton/RcButton.vue';
6+
import { RcButtonType } from '@components/RcButton/types';
57
68
const { dropdownItems } = inject<DropdownContext>('dropdownContext') || defaultContext;
79
const store = useStore();
810
const unreadCount = computed<number>(() => store.getters['notifications/unreadCount']);
9-
const markAllReadButton = ref<HTMLElement>();
11+
const markAllReadButton = ref<RcButtonType | null>(null);
1012
1113
const markAllRead = (keyboard: boolean) => {
1214
store.dispatch('notifications/markAllRead');
1315
14-
// If we have focus, then move to the next item if activated by the keyboard
15-
if (keyboard && document.activeElement === markAllReadButton?.value) {
16+
// If activated via keyboard, move focus to the next dropdown item
17+
if (keyboard) {
1618
moveFocus(true);
1719
}
1820
};
@@ -65,18 +67,19 @@ const gotFocus = (e: Event) => {
6567
{{ t('notificationCenter.title') }}
6668
</div>
6769
<div v-if="unreadCount !== 0">
68-
<a
70+
<RcButton
6971
ref="markAllReadButton"
70-
role="button"
72+
variant="ghost"
73+
size="small"
7174
tabindex="-1"
72-
href="#"
75+
class="mark-all-read"
7376
data-testid="notifications-center-markall-read"
7477
@keydown.up.down.stop.prevent="handleKeydown"
7578
@keydown.enter.space.stop="markAllRead(true)"
7679
@click="markAllRead(false)"
7780
>
7881
{{ t('notificationCenter.markAllRead') }}
79-
</a>
82+
</RcButton>
8083
</div>
8184
</div>
8285
<div class="notification-border" />
@@ -104,8 +107,17 @@ const gotFocus = (e: Event) => {
104107
flex: 1;
105108
}
106109
107-
A {
110+
.mark-all-read {
111+
padding: 0;
112+
min-height: auto;
113+
font-size: inherit;
114+
line-height: inherit;
108115
color: var(--link);
116+
117+
&:hover {
118+
color: var(--body-text);
119+
text-decoration: underline;
120+
}
109121
}
110122
}
111123
}
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import { ref } from 'vue';
2+
import { mount, shallowMount } from '@vue/test-utils';
3+
import NotificationHeader from '@shell/components/nav/NotificationCenter/NotificationHeader.vue';
4+
import { defaultContext } from '@components/RcDropdown/types';
5+
6+
const buildStore = (unreadCount = 1) => {
7+
const dispatch = jest.fn();
8+
const store = {
9+
dispatch,
10+
getters: { 'notifications/unreadCount': unreadCount },
11+
};
12+
13+
return { store, dispatch };
14+
};
15+
16+
const buildGlobal = (store: any) => ({
17+
provide: {
18+
store,
19+
dropdownContext: { ...defaultContext, dropdownItems: ref<HTMLElement[]>([]) },
20+
},
21+
mocks: { $store: store },
22+
});
23+
24+
jest.mock('vuex', () => ({ useStore: () => (globalThis as any).__testStore }));
25+
26+
describe('component: NotificationHeader', () => {
27+
afterEach(() => {
28+
(globalThis as any).__testStore = undefined;
29+
});
30+
31+
it('renders the mark all read action when there are unread notifications', () => {
32+
const { store } = buildStore(3);
33+
34+
(globalThis as any).__testStore = store;
35+
36+
const wrapper = shallowMount(NotificationHeader, { global: buildGlobal(store) });
37+
38+
expect(wrapper.find('[data-testid="notifications-center-markall-read"]').exists()).toBe(true);
39+
});
40+
41+
it('hides the mark all read action when there are no unread notifications', () => {
42+
const { store } = buildStore(0);
43+
44+
(globalThis as any).__testStore = store;
45+
46+
const wrapper = shallowMount(NotificationHeader, { global: buildGlobal(store) });
47+
48+
expect(wrapper.find('[data-testid="notifications-center-markall-read"]').exists()).toBe(false);
49+
});
50+
51+
it('dispatches notifications/markAllRead when clicked', async() => {
52+
const { store, dispatch } = buildStore(2);
53+
54+
(globalThis as any).__testStore = store;
55+
56+
const wrapper = mount(NotificationHeader, { global: buildGlobal(store) });
57+
58+
await wrapper.find('[data-testid="notifications-center-markall-read"]').trigger('click');
59+
60+
expect(dispatch).toHaveBeenCalledWith('notifications/markAllRead');
61+
});
62+
63+
// Regression test for https://github.com/rancher/dashboard/issues/16923
64+
// "Mark all as read" was originally an <a href="#"> which, on click, navigated
65+
// to "#" and stripped any existing URL hash fragment (e.g. #pod). Rendering it
66+
// as a <button> (via RcButton) removes the default navigation behavior entirely,
67+
// so the URL hash is preserved and extensions scoped via LocationConfig.hash
68+
// continue to match after activation.
69+
it('renders mark all read as a <button> so activating it cannot strip the URL hash', () => {
70+
const { store } = buildStore(2);
71+
72+
(globalThis as any).__testStore = store;
73+
74+
const wrapper = mount(NotificationHeader, { global: buildGlobal(store) });
75+
const markAll = wrapper.find('[data-testid="notifications-center-markall-read"]');
76+
77+
expect(markAll.element.tagName).toBe('BUTTON');
78+
expect(markAll.attributes('href')).toBeUndefined();
79+
});
80+
});

0 commit comments

Comments
 (0)