Skip to content

Commit 920680f

Browse files
authored
Fix incorrect button sizes introduced by RcButton changes (#17178)
* Fixing some of the more notable size discrepencies with RcButton Fixes #16755 * Removed the change to the RcButton default size. * An attempt at wrapping the RcButton with a link to prevent regression. * Provided a convenience prop in RcButton to support links shaped like buttons This is almost entirely based on @rak-phillip 's work at ab4abcf * Reintroducing the role attribute * Updated storybook * Found an area where we had additional margin being adding in the sortable table bulk actions. * Fixing a size issue I noticed with the notifcation button. This was likely caused by missing changes we made to the rcbutton default size.
1 parent ed00bf8 commit 920680f

17 files changed

Lines changed: 230 additions & 79 deletions

File tree

cypress/e2e/po/components/create-edit-view.po.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export default class CreateEditViewPo extends ComponentPo {
2424
}
2525

2626
cancel() {
27-
return new AsyncButtonPo(this.self().find('.cru-resource-footer .role-secondary')).click();
27+
return new AsyncButtonPo(this.self().find('.cru-resource-footer .variant-secondary')).click();
2828
}
2929

3030
saveAndWait() {

cypress/e2e/po/pages/explorer/charts/install-charts.po.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export class InstallChartPage extends PagePo {
2323
}
2424

2525
nextPage() {
26-
const btn = new AsyncButtonPo('.controls-steps .btn.role-primary');
26+
const btn = new AsyncButtonPo('.controls-steps .btn.variant-primary');
2727

2828
btn.click(true);
2929

cypress/e2e/po/pages/explorer/project-secrets.po.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export class ProjectSecretsListPagePo extends BaseListPagePo {
2727
}
2828

2929
createButtonTitle() {
30-
return this.createButton().invoke('text');
30+
return this.createButton().invoke('text').invoke('trim');
3131
}
3232
}
3333

cypress/e2e/po/pages/explorer/secrets.po.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export class SecretsListPagePo extends BaseListPagePo {
3535
}
3636

3737
createButtonTitle() {
38-
return this.createButton().invoke('text');
38+
return this.createButton().invoke('text').invoke('trim');
3939
}
4040

4141
list() {

pkg/rancher-components/src/components/RcButton/RcButton.test.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { mount } from '@vue/test-utils';
1+
import { mount, RouterLinkStub } from '@vue/test-utils';
22
import RcButton from './RcButton.vue';
33

44
describe('rcButton.vue', () => {
@@ -138,4 +138,40 @@ describe('rcButton.vue', () => {
138138
expect(button.classes()).toContain('variant-ghost');
139139
});
140140
});
141+
142+
describe('to prop', () => {
143+
it('renders as a <button> when no "to" prop is provided', () => {
144+
const wrapper = mount(RcButton);
145+
146+
expect(wrapper.find('button').exists()).toBe(true);
147+
expect(wrapper.findComponent(RouterLinkStub).exists()).toBe(false);
148+
});
149+
150+
it('renders as a RouterLink when "to" prop is provided', () => {
151+
const to = { name: 'some-route' };
152+
const wrapper = mount(RcButton, {
153+
props: { to },
154+
global: { stubs: { RouterLink: RouterLinkStub } },
155+
});
156+
157+
const link = wrapper.findComponent(RouterLinkStub);
158+
159+
expect(link.exists()).toBe(true);
160+
expect(wrapper.find('button').exists()).toBe(false);
161+
expect(link.props('to')).toStrictEqual(to);
162+
});
163+
164+
it('applies button classes when rendered as a RouterLink', () => {
165+
const wrapper = mount(RcButton, {
166+
props: { to: '/foo', variant: 'secondary' },
167+
global: { stubs: { RouterLink: RouterLinkStub } },
168+
});
169+
170+
const link = wrapper.findComponent(RouterLinkStub);
171+
172+
expect(link.classes()).toContain('rc-button');
173+
expect(link.classes()).toContain('btn');
174+
expect(link.classes()).toContain('variant-secondary');
175+
});
176+
});
141177
});

pkg/rancher-components/src/components/RcButton/RcButton.vue

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,15 @@
77
*
88
* <rc-button variant="primary" @click="doAction">Perform an Action</rc-button>
99
*/
10-
import { computed, ref } from 'vue';
10+
import { computed, ref, resolveComponent } from 'vue';
11+
import type { RouteLocationRaw } from 'vue-router';
1112
import {
12-
ButtonVariantProps, ButtonSizeProps, ButtonVariantNewProps, ButtonSizeNewProps, ButtonSize,
13-
IconProps
13+
ButtonVariantProps,
14+
ButtonSizeProps,
15+
ButtonVariantNewProps,
16+
ButtonSizeNewProps,
17+
ButtonSize,
18+
IconProps,
1419
} from './types';
1520
import RcIcon from '@components/RcIcon/RcIcon.vue';
1621
@@ -33,7 +38,22 @@ const buttonSizesNew: { size: ButtonSize, className: string }[] = [
3338
{ size: 'large', className: 'btn-large' },
3439
];
3540
36-
const props = withDefaults(defineProps<ButtonVariantProps & ButtonSizeProps & ButtonVariantNewProps & ButtonSizeNewProps & IconProps>(), { size: 'medium' });
41+
const props = withDefaults(
42+
defineProps<
43+
ButtonVariantProps &
44+
ButtonSizeProps &
45+
ButtonVariantNewProps &
46+
ButtonSizeNewProps &
47+
IconProps & { to?: RouteLocationRaw }
48+
>(),
49+
{
50+
size: 'medium',
51+
to: undefined,
52+
}
53+
);
54+
55+
const tag = computed(() => (props.to ? resolveComponent('RouterLink') : 'button'));
56+
const role = computed(() => (props.to ? 'link' : 'button'));
3757
3858
const activeVariantClassName = computed(() => {
3959
if (props.variant === 'multiAction' || props.multiAction) {
@@ -94,9 +114,11 @@ defineExpose({ focus });
94114
</script>
95115

96116
<template>
97-
<button
117+
<component
118+
:is="tag"
98119
ref="RcFocusTarget"
99-
role="button"
120+
:role="role"
121+
:to="to"
100122
:class="{ ...buttonClass }"
101123
>
102124
<slot
@@ -124,15 +146,23 @@ defineExpose({ focus });
124146
size="inherit"
125147
/>
126148
</slot>
127-
</button>
149+
</component>
128150
</template>
129151

130152
<style lang="scss" scoped>
131-
button {
153+
.rc-button {
132154
display: inline-flex;
133155
align-items: center;
134156
justify-content: center;
135157
158+
// Override global .btn > .icon:not(:only-child) { margin-right: 6px } from _button.scss.
159+
// RcButton uses flex gap for spacing instead. The :not(:only-child) clause matches the global
160+
// selector so we win on specificity regardless of stylesheet load order; :deep() is needed to
161+
// target slotted content.
162+
& > :deep(.icon:not(:only-child)) {
163+
margin-right: 0;
164+
}
165+
136166
// Much of the styling in here came from _button.scss. Because we're making changes from role to variant and we don't want to impact existing use cases we're pulling in some of these styles. We should in the long run deprecate that file.
137167
// Variant styles
138168
&.variant-primary {

pkg/rancher-components/src/components/RcDropdown/RcDropdownTrigger.vue

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,18 +35,20 @@ defineExpose({ focus });
3535
@keydown.enter.space="handleKeydown"
3636
@click="showMenu(true)"
3737
>
38-
<template #before>
39-
<slot name="before">
40-
<!-- Empty Content -->
41-
</slot>
38+
<template
39+
v-if="$slots.before"
40+
#before
41+
>
42+
<slot name="before" />
4243
</template>
4344
<slot name="default">
4445
<!--Empty slot content-->
4546
</slot>
46-
<template #after>
47-
<slot name="after">
48-
<!-- Empty Content -->
49-
</slot>
47+
<template
48+
v-if="$slots.after"
49+
#after
50+
>
51+
<slot name="after" />
5052
</template>
5153
</RcButton>
5254
</template>

shell/components/ActionDropdownShell.vue

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,15 @@ const setBulkActionOfInterest = (act: HiddenAction | null, event: 'mouseover' |
3535
>
3636
<rc-dropdown-trigger
3737
class="bulk-actions-dropdown"
38+
size="large"
3839
:disabled="disabled"
3940
>
4041
<template #before>
4142
<i class="icon icon-gear" />
4243
</template>
4344
<span>{{ t('sortableTable.bulkActions.collapsed.label') }}</span>
4445
<template #after>
45-
<i class="ml-10 icon icon-chevron-down" />
46+
<i class="icon icon-chevron-down" />
4647
</template>
4748
</rc-dropdown-trigger>
4849
<template #dropdownCollection>

shell/components/CruResourceFooter.vue

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,16 @@ import { mapGetters } from 'vuex';
33
44
import AsyncButton from '@shell/components/AsyncButton';
55
import ResourceCancelModal from '@shell/components/ResourceCancelModal';
6+
import { RcButton } from '@components/RcButton';
67
import { _VIEW } from '@shell/config/query-params';
78
89
export default {
910
emits: ['cancel-confirmed', 'finish'],
1011
11-
components: { AsyncButton, ResourceCancelModal },
12-
props: {
12+
components: {
13+
AsyncButton, RcButton, ResourceCancelModal
14+
},
15+
props: {
1316
mode: {
1417
type: String,
1518
default: 'create',
@@ -84,16 +87,17 @@ export default {
8487
<div class="cru-resource-footer">
8588
<slot name="footer-prefix" />
8689
<slot name="cancel">
87-
<button
90+
<RcButton
8891
v-if="!isView && showCancel"
8992
id="cru-cancel"
9093
:data-testid="componentTestid + '-cancel'"
9194
type="button"
92-
class="btn role-secondary"
95+
variant="secondary"
96+
size="large"
9397
@click="confirmCancelRequired ? checkCancel(true) : $emit('cancel-confirmed', true)"
9498
>
9599
<t k="generic.cancel" />
96-
</button>
100+
</RcButton>
97101
</slot>
98102
<slot :checkCancel="checkCancel">
99103
<AsyncButton

shell/components/Resource/Detail/TitleBar/index.vue

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -167,12 +167,6 @@ const showAdditionalActionButtons = computed(() => isArray(additionalActions));
167167
position: relative;
168168
}
169169
170-
.icon-document {
171-
width: 15px;
172-
font-size: 16px;
173-
margin-right: 10px;
174-
}
175-
176170
.actions {
177171
display: flex;
178172
align-items: center;

0 commit comments

Comments
 (0)