diff --git a/ee/app/assets/javascripts/approvals/components/approval_rules_app.vue b/ee/app/assets/javascripts/approvals/components/approval_rules_app.vue index 2a4b16a20f342cf39eaa52552eaaea8ddf0bcdc6..64b1544cb5b2733a23c40caa1135b692e904da1e 100644 --- a/ee/app/assets/javascripts/approvals/components/approval_rules_app.vue +++ b/ee/app/assets/javascripts/approvals/components/approval_rules_app.vue @@ -99,7 +99,6 @@ export default { > <template v-if="canAddApprovalRule" #actions> <gl-button - :class="{ 'gl-mr-3': targetBranch, 'gl-mr-0': !targetBranch }" :disabled="isLoading" category="secondary" size="small" diff --git a/ee/app/assets/javascripts/approvals/mr_edit/free_tier_promo.vue b/ee/app/assets/javascripts/approvals/mr_edit/free_tier_promo.vue index 8d519e02b15fcdf93c25149cf67399b47e828522..35595782bb81bfe023daff439a25de556b0a3b05 100644 --- a/ee/app/assets/javascripts/approvals/mr_edit/free_tier_promo.vue +++ b/ee/app/assets/javascripts/approvals/mr_edit/free_tier_promo.vue @@ -1,6 +1,7 @@ <script> -import { GlButton, GlLink, GlCollapse, GlCard } from '@gitlab/ui'; +import { GlBanner, GlButton, GlLink, GlCollapse } from '@gitlab/ui'; import Tracking from '~/tracking'; +import { s__ } from '~/locale'; import LocalStorageSync from '~/vue_shared/components/local_storage_sync.vue'; import { MR_APPROVALS_PROMO_DISMISSED, @@ -12,11 +13,11 @@ const trackingMixin = Tracking.mixin({}); export default { components: { + GlBanner, GlButton, GlLink, LocalStorageSync, GlCollapse, - GlCard, }, mixins: [trackingMixin], inject: ['learnMorePath', 'promoImageAlt', 'promoImagePath', 'tryNowPath'], @@ -24,20 +25,24 @@ export default { return { // isReady - used to render components after local storage has synced isReady: false, - // userManuallyCollapsed - set to true if the collapsible is collapsed - userManuallyCollapsed: false, // isExpanded - the current collapsible state isExpanded: true, + // isBannerVisible - is the banner visible + isBannerDismissed: false, }; }, computed: { icon() { return this.isExpanded ? 'chevron-down' : 'chevron-right'; }, - }, - watch: { - userManuallyCollapsed(isCollapsed) { - this.isExpanded = !isCollapsed; + buttonAttributes() { + return { + target: '_blank', + 'aria-label': s__('ApprovalRule|Learn more about merge request approval rules'), + 'data-track-action': this.$options.trackingEvents.tryNowClick.action, + 'data-track-label': this.$options.trackingEvents.tryNowClick.label, + 'data-testid': 'promo-dismiss-btn', + }; }, }, mounted() { @@ -50,8 +55,6 @@ export default { toggleCollapse() { // If we're expanded already, then the user tried to collapse... if (this.isExpanded) { - this.userManuallyCollapsed = true; - const { action, ...options } = MR_APPROVALS_PROMO_TRACKING_EVENTS.collapsePromo; this.track(action, options); } else { @@ -61,6 +64,10 @@ export default { this.isExpanded = !this.isExpanded; }, + hideBanner() { + this.isBannerDismissed = true; + this.isExpanded = false; + }, }, trackingEvents: MR_APPROVALS_PROMO_TRACKING_EVENTS, i18n: MR_APPROVALS_PROMO_I18N, @@ -71,55 +78,53 @@ export default { <template> <div class="gl-mt-2"> <local-storage-sync - v-model="userManuallyCollapsed" + v-model="isBannerDismissed" :storage-key="$options.MR_APPROVALS_PROMO_DISMISSED" /> <template v-if="isReady"> - <p class="gl-mb-0 gl-text-gray-500"> + <p class="gl-mb-0 gl-text-subtle"> {{ $options.i18n.summary }} </p> - <gl-button variant="link" :icon="icon" data-testid="collapse-btn" @click="toggleCollapse"> + <gl-button + v-if="!isBannerDismissed" + variant="link" + :icon="icon" + data-testid="collapse-btn" + @click="toggleCollapse" + > {{ $options.i18n.accordionTitle }} </gl-button> - <gl-collapse v-model="isExpanded"> - <gl-card class="gl-new-card" data-testid="mr-approval-rules"> - <div class="gl-flex gl-items-start gl-gap-6"> - <img :src="promoImagePath" :alt="promoImageAlt" class="svg" /> + <gl-collapse v-if="!isBannerDismissed" v-model="isExpanded"> + <gl-banner + :title="$options.i18n.promoTitle" + :svg-path="promoImagePath" + :button-text="$options.i18n.tryNow" + :button-link="tryNowPath" + :button-attributes="buttonAttributes" + class="gl-mt-3" + data-testid="mr-approval-rules" + @close="hideBanner" + > + <ul class="gl-mb-5 gl-list-inside gl-p-0"> + <li v-for="(statement, index) in $options.i18n.valueStatements" :key="index"> + {{ statement }} + </li> + </ul> - <div class="gl-grow"> - <h4 class="gl-mb-3 gl-mt-0 gl-text-base gl-leading-20"> - {{ $options.i18n.promoTitle }} - </h4> - <ul class="gl-mb-3 gl-list-inside gl-p-0"> - <li v-for="(statement, index) in $options.i18n.valueStatements" :key="index"> - {{ statement }} - </li> - </ul> - <div class="gl-flex gl-items-center gl-gap-4"> - <gl-button - category="primary" - variant="confirm" - :href="tryNowPath" - target="_blank" - :aria-label="s__('ApprovalRule|Learn more about merge request approval rules')" - :data-track-action="$options.trackingEvents.tryNowClick.action" - :data-track-label="$options.trackingEvents.tryNowClick.label" - >{{ $options.i18n.tryNow }}</gl-button - > - <gl-link - :href="learnMorePath" - target="_blank" - :data-track-action="$options.trackingEvents.learnMoreClick.action" - :data-track-label="$options.trackingEvents.learnMoreClick.label" - > - {{ $options.i18n.learnMore }} - </gl-link> - </div> - </div> - </div> - </gl-card> + <template #actions> + <gl-link + :href="learnMorePath" + target="_blank" + class="gl-ml-3" + :data-track-action="$options.trackingEvents.learnMoreClick.action" + :data-track-label="$options.trackingEvents.learnMoreClick.label" + > + {{ $options.i18n.learnMore }} + </gl-link> + </template> + </gl-banner> </gl-collapse> </template> </div> diff --git a/ee/spec/frontend/approvals/mr_edit/free_tier_promo_spec.js b/ee/spec/frontend/approvals/mr_edit/free_tier_promo_spec.js index ddb289264371c9d69b481044b16ec8201a3a654d..84ce757ad87e52982183189fbd5c6d222c466716 100644 --- a/ee/spec/frontend/approvals/mr_edit/free_tier_promo_spec.js +++ b/ee/spec/frontend/approvals/mr_edit/free_tier_promo_spec.js @@ -1,4 +1,4 @@ -import { GlButton, GlLink, GlCollapse } from '@gitlab/ui'; +import { GlBanner, GlButton, GlLink, GlCollapse } from '@gitlab/ui'; import { nextTick } from 'vue'; import { shallowMountExtended, extendedWrapper } from 'helpers/vue_test_utils_helper'; import { useLocalStorageSpy } from 'helpers/local_storage_helper'; @@ -36,6 +36,7 @@ describe('FreeTierPromo component', () => { }, stubs: { LocalStorageSync, + GlBanner, }, }); trackingSpy = mockTracking(undefined, wrapper.element, jest.spyOn); @@ -45,6 +46,7 @@ describe('FreeTierPromo component', () => { const findCollapse = () => extendedWrapper(wrapper.findComponent(GlCollapse)); const findLearnMore = () => findCollapse().findComponent(GlLink); const findTryNow = () => findCollapse().findComponent(GlButton); + const findBanner = () => wrapper.findComponent(GlBanner); afterEach(() => { unmockTracking(); @@ -129,9 +131,7 @@ describe('FreeTierPromo component', () => { }); it('shows the promo image', () => { - const promoImage = findCollapse().findByAltText('some promo image'); - - expect(promoImage.element.src).toBe('/some-image.svg'); + expect(findBanner().props('svgPath')).toBe('/some-image.svg'); }); }); @@ -148,14 +148,26 @@ describe('FreeTierPromo component', () => { expect(findCollapse().props('visible')).toBe(false); }); - it('updates local storage', () => { - expect(localStorage.setItem).toHaveBeenCalledWith(MR_APPROVALS_PROMO_DISMISSED, 'true'); - }); - it('updates button icon', () => { expect(findCollapseToggleButton().attributes('icon')).toBe(COLLAPSED_ICON); }); }); + + describe('when user dismisses banner', () => { + beforeEach(() => { + findBanner().vm.$emit('close'); + }); + + it('hides toggle, collapsible and banner', () => { + expect(findCollapseToggleButton().exists()).toBe(false); + expect(wrapper.findComponent(GlCollapse).exists()).toBe(false); + expect(findBanner().exists()).toBe(false); + }); + + it('updates local storage', () => { + expect(localStorage.setItem).toHaveBeenCalledWith(MR_APPROVALS_PROMO_DISMISSED, 'true'); + }); + }); }); describe('when local storage is initialized with mr_approvals_promo.dismissed=true', () => { @@ -166,30 +178,10 @@ describe('FreeTierPromo component', () => { localStorage.setItem.mockClear(); }); - it('should show collapse container as collapsed', () => { - expect(findCollapse().props('visible')).toBe(false); - }); - - describe('when user clicks collapse toggle', () => { - beforeEach(() => { - findCollapseToggleButton().vm.$emit('click'); - }); - - it('tracks intent to expand', () => { - expectTracking(undefined, trackingEvents.expandPromo); - }); - - it('expands the collapse component', () => { - expect(findCollapse().props('visible')).toBe(true); - }); - - it('does NOT update local storage', () => { - expect(localStorage.setItem).not.toHaveBeenCalled(); - }); - - it('updates button icon', () => { - expect(findCollapseToggleButton().attributes('icon')).toBe(EXPANDED_ICON); - }); + it("doesn't render toggle, collapsible and banner", () => { + expect(findCollapseToggleButton().exists()).toBe(false); + expect(wrapper.findComponent(GlCollapse).exists()).toBe(false); + expect(findBanner().exists()).toBe(false); }); }); });