Merge branch '22139-add-max-height-for-terms-of-service-panel' into 'master'

Convert terms of service to Vue and add max height

See merge request gitlab-org/gitlab!72325

app/assets/javascripts/flash.js

@@ -9,6 +9,8 @@ const FLASH_TYPES = {
   WARNING: 'warning',
 };
 
+const FLASH_CLOSED_EVENT = 'flashClosed';
+
 const getCloseEl = (flashEl) => {
   return flashEl.querySelector('.js-close-icon');
 };
@@ -26,6 +28,7 @@ const hideFlash = (flashEl, fadeTransition = true) => {
     () => {
       flashEl.remove();
       window.dispatchEvent(new Event('resize'));
+      flashEl.dispatchEvent(new Event(FLASH_CLOSED_EVENT));
       if (document.body.classList.contains('flash-shown'))
         document.body.classList.remove('flash-shown');
     },
@@ -132,4 +135,5 @@ export {
   hideFlash,
   removeFlashClickListener,
   FLASH_TYPES,
+  FLASH_CLOSED_EVENT,
 };

app/assets/javascripts/pages/users/terms/index/index.js

+import { initTermsApp } from '~/terms';
+import { waitForCSSLoaded } from '~/helpers/startup_css_helper';
+
+waitForCSSLoaded(initTermsApp);

app/assets/javascripts/terms/components/app.vue

+<script>
+import $ from 'jquery';
+import { GlButton, GlIntersectionObserver, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui';
+
+import { FLASH_TYPES, FLASH_CLOSED_EVENT } from '~/flash';
+import { isLoggedIn } from '~/lib/utils/common_utils';
+import { __ } from '~/locale';
+import csrf from '~/lib/utils/csrf';
+import '~/behaviors/markdown/render_gfm';
+
+export default {
+  name: 'TermsApp',
+  i18n: {
+    accept: __('Accept terms'),
+    continue: __('Continue'),
+    decline: __('Decline and sign out'),
+  },
+  flashElements: [],
+  csrf,
+  directives: {
+    SafeHtml,
+  },
+  components: { GlButton, GlIntersectionObserver },
+  inject: ['terms', 'permissions', 'paths'],
+  data() {
+    return {
+      acceptDisabled: true,
+    };
+  },
+  computed: {
+    isLoggedIn,
+  },
+  mounted() {
+    this.renderGFM();
+    this.setScrollableViewportHeight();
+
+    this.$options.flashElements = [
+      ...document.querySelectorAll(
+        Object.values(FLASH_TYPES)
+          .map((flashType) => `.flash-${flashType}`)
+          .join(','),
+      ),
+    ];
+
+    this.$options.flashElements.forEach((flashElement) => {
+      flashElement.addEventListener(FLASH_CLOSED_EVENT, this.handleFlashClose);
+    });
+  },
+  beforeDestroy() {
+    this.$options.flashElements.forEach((flashElement) => {
+      flashElement.removeEventListener(FLASH_CLOSED_EVENT, this.handleFlashClose);
+    });
+  },
+  methods: {
+    renderGFM() {
+      $(this.$refs.gfmContainer).renderGFM();
+    },
+    handleBottomReached() {
+      this.acceptDisabled = false;
+    },
+    setScrollableViewportHeight() {
+      // Reset `max-height` inline style
+      this.$refs.scrollableViewport.style.maxHeight = '';
+
+      const { scrollHeight, clientHeight } = document.documentElement;
+
+      // Set `max-height` to 100vh minus all elements that are NOT the scrollable viewport (header, footer, alerts, etc)
+      this.$refs.scrollableViewport.style.maxHeight = `calc(100vh - ${
+        scrollHeight - clientHeight
+      }px)`;
+    },
+    handleFlashClose(event) {
+      this.setScrollableViewportHeight();
+      event.target.removeEventListener(FLASH_CLOSED_EVENT, this.handleFlashClose);
+    },
+  },
+};
+</script>
+
+<template>
+  <div>
+    <div class="gl-card-body gl-relative gl-pb-0 gl-px-0" data-qa-selector="terms_content">
+      <div
+        class="terms-fade gl-absolute gl-left-5 gl-right-5 gl-bottom-0 gl-h-11 gl-pointer-events-none"
+      ></div>
+      <div
+        ref="scrollableViewport"
+        data-testid="scrollable-viewport"
+        class="gl-h-100vh gl-overflow-y-auto gl-pb-11 gl-px-5"
+      >
+        <div ref="gfmContainer" v-safe-html="terms"></div>
+        <gl-intersection-observer @appear="handleBottomReached">
+          <div></div>
+        </gl-intersection-observer>
+      </div>
+    </div>
+    <div v-if="isLoggedIn" class="gl-card-footer gl-display-flex gl-justify-content-end">
+      <form v-if="permissions.canDecline" method="post" :action="paths.decline">
+        <gl-button type="submit">{{ $options.i18n.decline }}</gl-button>
+        <input :value="$options.csrf.token" type="hidden" name="authenticity_token" />
+      </form>
+      <form v-if="permissions.canAccept" class="gl-ml-3" method="post" :action="paths.accept">
+        <gl-button
+          type="submit"
+          variant="confirm"
+          :disabled="acceptDisabled"
+          data-qa-selector="accept_terms_button"
+          >{{ $options.i18n.accept }}</gl-button
+        >
+        <input :value="$options.csrf.token" type="hidden" name="authenticity_token" />
+      </form>
+      <gl-button v-else class="gl-ml-3" :href="paths.root" variant="confirm">{{
+        $options.i18n.continue
+      }}</gl-button>
+    </div>
+  </div>
+</template>

app/assets/javascripts/terms/index.js

+import Vue from 'vue';
+
+import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
+import TermsApp from './components/app.vue';
+
+export const initTermsApp = () => {
+  const el = document.getElementById('js-terms-of-service');
+
+  if (!el) return false;
+
+  const { terms, permissions, paths } = convertObjectPropsToCamelCase(
+    JSON.parse(el.dataset.termsData),
+    { deep: true },
+  );
+
+  return new Vue({
+    el,
+    provide: { terms, permissions, paths },
+    render(createElement) {
+      return createElement(TermsApp);
+    },
+  });
+};

app/assets/stylesheets/framework.scss

@@ -62,7 +62,6 @@
 @import 'framework/sortable';
 @import 'framework/ci_variable_list';
 @import 'framework/feature_highlight';
-@import 'framework/terms';
 @import 'framework/read_more';
 @import 'framework/flex_grid';
 @import 'framework/system_messages';

app/assets/stylesheets/framework/terms.scss → app/assets/stylesheets/page_bundles/terms.scss

+@import 'mixins_and_variables_and_functions';
+
 .terms {
+  .with-system-header &,
+  .with-system-header.with-performance-bar &,
   .with-performance-bar & {
     margin-top: 0;
   }
 
-  .alert-wrapper {
-    min-height: $header-height + $gl-padding;
+  .terms-fade {
+    background: linear-gradient(0deg, $white 0%, rgba($white, 0.5) 100%);
   }
 
   .content {
     padding-top: $gl-padding;
   }
 
-  .card {
-    .card-header {
+  .gl-card {
+    .gl-card-header {
       display: flex;
       align-items: center;
       justify-content: space-between;

app/helpers/terms_helper.rb

+# frozen_string_literal: true
+
+module TermsHelper
+  def terms_data(terms, redirect)
+    redirect_params = { redirect: redirect } if redirect
+
+    {
+      terms: markdown_field(terms, :terms),
+      permissions: {
+        can_accept: can?(current_user, :accept_terms, terms),
+        can_decline: can?(current_user, :decline_terms, terms)
+      },
+      paths: {
+        accept: accept_term_path(terms, redirect_params),
+        decline: decline_term_path(terms, redirect_params),
+        root: root_path
+      }
+    }.to_json
+  end
+end

app/views/layouts/terms.html.haml

 !!! 5
+- add_page_specific_style 'page_bundles/terms'
 - @hide_breadcrumbs = true
 %html{ lang: I18n.locale, class: page_class }
   = render "layouts/head"
 
   %body{ data: { page: body_data_page } }
     .layout-page.terms{ class: page_class }
-      .content-wrapper
+      .content-wrapper.gl-pb-5
         .mobile-overlay
         .alert-wrapper
           = render "layouts/broadcast"
           = render 'layouts/header/read_only_banner'
-          = render "layouts/flash", extra_flash_class: 'limit-container-width'
+          = render "layouts/flash"
 
         %div{ class: "#{container_class} limit-container-width" }
           .content{ id: "content-body" }
-            .card
-              .card-header
+            .gl-card
+              .gl-card-header
                 = brand_header_logo
                 - logo_text = brand_header_logo_type
                 - if logo_text.present?

app/views/users/terms/index.html.haml

 - redirect_params = { redirect: @redirect } if @redirect
 - accept_term_link = accept_term_path(@term, redirect_params)
 
-.card-body.rendered-terms{ data: { qa_selector: 'terms_content' } }
-  = markdown_field(@term, :terms)
-- if current_user
-  = render_if_exists 'devise/shared/form_phone_verification', accept_term_link: accept_term_link, inline: true
-  .card-footer.footer-block.clearfix
-    - if can?(current_user, :accept_terms, @term)
-      .float-right
-        = button_to accept_term_link, class: 'gl-button btn btn-confirm gl-ml-3', data: { qa_selector: 'accept_terms_button' } do
-          = _('Accept terms')
-    - else
-      .float-right
-        = link_to root_path, class: 'gl-button btn btn-confirm gl-ml-3' do
-          = _('Continue')
-    - if can?(current_user, :decline_terms, @term)
-      .float-right
-        = button_to decline_term_path(@term, redirect_params), class: 'gl-button btn btn-default gl-ml-3' do
-          =  _('Decline and sign out')
+- if Feature.enabled?(:terms_of_service_vue, current_user, default_enabled: :yaml)
+  #js-terms-of-service{ data: { terms_data: terms_data(@term, @redirect) } }
+- else
+  .card-body.rendered-terms{ data: { qa_selector: 'terms_content' } }
+    = markdown_field(@term, :terms)
+  - if current_user
+    = render_if_exists 'devise/shared/form_phone_verification', accept_term_link: accept_term_link, inline: true
+    .card-footer.footer-block.clearfix
+      - if can?(current_user, :accept_terms, @term)
+        .float-right
+          = button_to accept_term_link, class: 'gl-button btn btn-confirm gl-ml-3', data: { qa_selector: 'accept_terms_button' } do
+            = _('Accept terms')
+      - else
+        .float-right
+          = link_to root_path, class: 'gl-button btn btn-confirm gl-ml-3' do
+            = _('Continue')
+      - if can?(current_user, :decline_terms, @term)
+        .float-right
+          = button_to decline_term_path(@term, redirect_params), class: 'gl-button btn btn-default gl-ml-3' do
+            =  _('Decline and sign out')

config/application.rb

@@ -255,6 +255,7 @@ module Gitlab
     config.assets.precompile << "page_bundles/security_discover.css"
     config.assets.precompile << "page_bundles/signup.css"
     config.assets.precompile << "page_bundles/terminal.css"
+    config.assets.precompile << "page_bundles/terms.css"
     config.assets.precompile << "page_bundles/todos.css"
     config.assets.precompile << "page_bundles/wiki.css"
     config.assets.precompile << "page_bundles/xterm.css"

config/feature_flags/development/terms_of_service_vue.yml

+---
+name: terms_of_service_vue
+introduced_by_url:
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/343046
+milestone: '14.5'
+type: development
+group: group::access
+default_enabled: false

ee/app/assets/stylesheets/framework/terms.scss

-.terms {
-  .with-system-header &,
-  .with-system-header.with-performance-bar & {
-    margin-top: 0;
-  }
-}

spec/features/users/login_spec.rb

@@ -753,7 +753,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_shared_state do
     end
   end
 
-  context 'when terms are enforced' do
+  context 'when terms are enforced', :js do
     let(:user) { create(:user) }
 
     before do
@@ -802,7 +802,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_shared_state do
       end
 
       context 'when the user did not enable 2FA' do
-        it 'asks to set 2FA before asking to accept the terms', :js do
+        it 'asks to set 2FA before asking to accept the terms' do
           expect(authentication_metrics)
             .to increment(:user_authenticated_counter)
 
@@ -887,7 +887,7 @@ RSpec.describe 'Login', :clean_gitlab_redis_shared_state do
       end
     end
 
-    context 'when the user does not have an email configured', :js do
+    context 'when the user does not have an email configured' do
       let(:user) { create(:omniauth_user, extern_uid: 'my-uid', provider: 'saml', email: 'temp-email-for-oauth-user@gitlab.localhost') }
 
       before do

spec/features/users/terms_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe 'Users > Terms' do
+RSpec.describe 'Users > Terms', :js do
   include TermsHelper
 
   let!(:term) { create(:term, terms: 'By accepting, you promise to be nice!') }

spec/frontend/flash_spec.js

@@ -3,6 +3,7 @@ import createFlash, {
   createAction,
   hideFlash,
   removeFlashClickListener,
+  FLASH_CLOSED_EVENT,
 } from '~/flash';
 
 describe('Flash', () => {
@@ -79,6 +80,16 @@ describe('Flash', () => {
 
       expect(el.remove.mock.calls.length).toBe(1);
     });
+
+    it(`dispatches ${FLASH_CLOSED_EVENT} event after transitionend event`, () => {
+      jest.spyOn(el, 'dispatchEvent');
+
+      hideFlash(el);
+
+      el.dispatchEvent(new Event('transitionend'));
+
+      expect(el.dispatchEvent).toHaveBeenCalledWith(new Event(FLASH_CLOSED_EVENT));
+    });
   });
 
   describe('createAction', () => {

spec/frontend/terms/components/app_spec.js

+import $ from 'jquery';
+import { merge } from 'lodash';
+import { GlIntersectionObserver } from '@gitlab/ui';
+import { nextTick } from 'vue';
+
+import { mountExtended } from 'helpers/vue_test_utils_helper';
+import { FLASH_TYPES, FLASH_CLOSED_EVENT } from '~/flash';
+import { isLoggedIn } from '~/lib/utils/common_utils';
+import TermsApp from '~/terms/components/app.vue';
+
+jest.mock('~/lib/utils/csrf', () => ({ token: 'mock-csrf-token' }));
+jest.mock('~/lib/utils/common_utils');
+
+describe('TermsApp', () => {
+  let wrapper;
+  let renderGFMSpy;
+
+  const defaultProvide = {
+    terms: 'foo bar',
+    paths: {
+      accept: '/-/users/terms/1/accept',
+      decline: '/-/users/terms/1/decline',
+      root: '/',
+    },
+    permissions: {
+      canAccept: true,
+      canDecline: true,
+    },
+  };
+
+  const createComponent = (provide = {}) => {
+    wrapper = mountExtended(TermsApp, {
+      provide: merge({}, defaultProvide, provide),
+    });
+  };
+
+  beforeEach(() => {
+    renderGFMSpy = jest.spyOn($.fn, 'renderGFM');
+    isLoggedIn.mockReturnValue(true);
+  });
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  const findFormWithAction = (path) => wrapper.find(`form[action="${path}"]`);
+  const findButton = (path) => findFormWithAction(path).find('button[type="submit"]');
+  const findScrollableViewport = () => wrapper.findByTestId('scrollable-viewport');
+
+  const expectFormWithSubmitButton = (buttonText, path) => {
+    const form = findFormWithAction(path);
+    const submitButton = findButton(path);
+
+    expect(form.exists()).toBe(true);
+    expect(submitButton.exists()).toBe(true);
+    expect(submitButton.text()).toBe(buttonText);
+    expect(
+      form
+        .find('input[type="hidden"][name="authenticity_token"][value="mock-csrf-token"]')
+        .exists(),
+    ).toBe(true);
+  };
+
+  it('renders terms of service as markdown', () => {
+    createComponent();
+
+    expect(wrapper.findByText(defaultProvide.terms).exists()).toBe(true);
+    expect(renderGFMSpy).toHaveBeenCalled();
+  });
+
+  describe('accept button', () => {
+    it('is disabled until user scrolls to the bottom of the terms', async () => {
+      createComponent();
+
+      expect(findButton(defaultProvide.paths.accept).attributes('disabled')).toBe('disabled');
+
+      wrapper.find(GlIntersectionObserver).vm.$emit('appear');
+
+      await nextTick();
+
+      expect(findButton(defaultProvide.paths.accept).attributes('disabled')).toBeUndefined();
+    });
+
+    describe('when user has permissions to accept', () => {
+      it('renders form and button to accept terms', () => {
+        createComponent();
+
+        expectFormWithSubmitButton(TermsApp.i18n.accept, defaultProvide.paths.accept);
+      });
+    });
+
+    describe('when user does not have permissions to accept', () => {
+      it('renders continue button', () => {
+        createComponent({ permissions: { canAccept: false } });
+
+        expect(wrapper.findByText(TermsApp.i18n.continue).exists()).toBe(true);
+      });
+    });
+  });
+
+  describe('decline button', () => {
+    describe('when user has permissions to decline', () => {
+      it('renders form and button to decline terms', () => {
+        createComponent();
+
+        expectFormWithSubmitButton(TermsApp.i18n.decline, defaultProvide.paths.decline);
+      });
+    });
+
+    describe('when user does not have permissions to decline', () => {
+      it('does not render decline button', () => {
+        createComponent({ permissions: { canDecline: false } });
+
+        expect(wrapper.findByText(TermsApp.i18n.decline).exists()).toBe(false);
+      });
+    });
+  });
+
+  it('sets height of scrollable viewport', () => {
+    jest.spyOn(document.documentElement, 'scrollHeight', 'get').mockImplementation(() => 800);
+    jest.spyOn(document.documentElement, 'clientHeight', 'get').mockImplementation(() => 600);
+
+    createComponent();
+
+    expect(findScrollableViewport().attributes('style')).toBe('max-height: calc(100vh - 200px);');
+  });
+
+  describe('when flash is closed', () => {
+    let flashEl;
+
+    beforeEach(() => {
+      flashEl = document.createElement('div');
+      flashEl.classList.add(`flash-${FLASH_TYPES.ALERT}`);
+      document.body.appendChild(flashEl);
+    });
+
+    afterEach(() => {
+      document.body.innerHTML = '';
+    });
+
+    it('recalculates height of scrollable viewport', () => {
+      jest.spyOn(document.documentElement, 'scrollHeight', 'get').mockImplementation(() => 800);
+      jest.spyOn(document.documentElement, 'clientHeight', 'get').mockImplementation(() => 600);
+
+      createComponent();
+
+      expect(findScrollableViewport().attributes('style')).toBe('max-height: calc(100vh - 200px);');
+
+      jest.spyOn(document.documentElement, 'scrollHeight', 'get').mockImplementation(() => 700);
+      jest.spyOn(document.documentElement, 'clientHeight', 'get').mockImplementation(() => 600);
+
+      flashEl.dispatchEvent(new Event(FLASH_CLOSED_EVENT));
+
+      expect(findScrollableViewport().attributes('style')).toBe('max-height: calc(100vh - 100px);');
+    });
+  });
+
+  describe('when user is signed out', () => {
+    beforeEach(() => {
+      isLoggedIn.mockReturnValue(false);
+    });
+
+    it('does not show any buttons', () => {
+      createComponent();
+
+      expect(wrapper.findByText(TermsApp.i18n.accept).exists()).toBe(false);
+      expect(wrapper.findByText(TermsApp.i18n.decline).exists()).toBe(false);
+      expect(wrapper.findByText(TermsApp.i18n.continue).exists()).toBe(false);
+    });
+  });
+});

spec/helpers/terms_helper_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe TermsHelper do
+  let_it_be(:current_user) { build(:user) }
+  let_it_be(:terms) { build(:term) }
+
+  before do
+    allow(helper).to receive(:current_user).and_return(current_user)
+  end
+
+  describe '#terms_data' do
+    let_it_be(:redirect) { '%2F' }
+    let_it_be(:terms_markdown) { 'Lorem ipsum dolor sit amet' }
+    let_it_be(:accept_path) { '/-/users/terms/14/accept?redirect=%2F' }
+    let_it_be(:decline_path) { '/-/users/terms/14/decline?redirect=%2F' }
+
+    subject(:result) { Gitlab::Json.parse(helper.terms_data(terms, redirect)) }
+
+    it 'returns correct json' do
+      expect(helper).to receive(:markdown_field).with(terms, :terms).and_return(terms_markdown)
+      expect(helper).to receive(:can?).with(current_user, :accept_terms, terms).and_return(true)
+      expect(helper).to receive(:can?).with(current_user, :decline_terms, terms).and_return(true)
+      expect(helper).to receive(:accept_term_path).with(terms, { redirect: redirect }).and_return(accept_path)
+      expect(helper).to receive(:decline_term_path).with(terms, { redirect: redirect }).and_return(decline_path)
+
+      expected = {
+        terms: terms_markdown,
+        permissions: {
+          can_accept: true,
+          can_decline: true
+        },
+        paths: {
+          accept: accept_path,
+          decline: decline_path,
+          root: root_path
+        }
+      }.as_json
+
+      expect(result).to eq(expected)
+    end
+  end
+end