[go: up one dir, main page]

Rewrite ShortcutsBlob functionality to use Vue component's lifecycle

The following discussion from !150836 (merged) should be addressed:

  • @markrian started a discussion: (+2 comments)

    suggestion: WDYT of relying on regular Vue event handling for this?

    Also, given that this is all ShortcutsBlob does, WDYT of wiring this up in this component's lifecycle? I consider Shortcuts* classes to be legacy, and I'd like to see it eventually be removed.

    Here's a patch with what I have in mind:

    diff --git a/app/assets/javascripts/repository/components/blob_controls.vue b/app/assets/javascripts/repository/components/blob_controls.vue
    index 5571796cd922..b256474d86e1 100644
    --- a/app/assets/javascripts/repository/components/blob_controls.vue
    +++ b/app/assets/javascripts/repository/components/blob_controls.vue
    @@ -5,10 +5,8 @@ import { createAlert } from '~/alert';
     import getRefMixin from '~/repository/mixins/get_ref';
     import initSourcegraph from '~/sourcegraph';
     import Shortcuts from '~/behaviors/shortcuts/shortcuts';
    -import { addShortcutsExtension } from '~/behaviors/shortcuts';
     import { shouldDisableShortcuts } from '~/behaviors/shortcuts/shortcuts_toggle';
    -import ShortcutsBlob from '~/behaviors/shortcuts/shortcuts_blob';
    -import { shortcircuitPermalinkButton } from '~/blob/utils';
    +import { moveToFilePermalink } from '~/blob/utils';
     import BlobLinePermalinkUpdater from '~/blob/blob_line_permalink_updater';
     import {
       keysFor,
    @@ -16,6 +14,7 @@ import {
       PROJECT_FILES_GO_TO_PERMALINK,
     } from '~/behaviors/shortcuts/keybindings';
     import { sanitize } from '~/lib/dompurify';
    +import { Mousetrap } from '~/lib/mousetrap';
     import { updateElementsVisibility } from '../utils/dom';
     import blobControlsQuery from '../queries/blob_controls.query.graphql';
     
    @@ -127,16 +126,18 @@ export default {
         blobInfo() {
           initSourcegraph();
           this.$nextTick(() => {
    -        this.initShortcuts();
             this.initLinksUpdate();
           });
         },
       },
    +  mounted() {
    +    Mousetrap.bind(keysFor(PROJECT_FILES_GO_TO_PERMALINK), this.moveToFilePermalink);
    +  },
    +  beforeDestroy() {
    +    Mousetrap.unbind(keysFor(PROJECT_FILES_GO_TO_PERMALINK), this.moveToFilePermalink);
    +  },
       methods: {
    -    initShortcuts() {
    -      shortcircuitPermalinkButton();
    -      addShortcutsExtension(ShortcutsBlob);
    -    },
    +    moveToFilePermalink,
         initLinksUpdate() {
           // eslint-disable-next-line no-new
           new BlobLinePermalinkUpdater(
    @@ -184,6 +185,7 @@ export default {
           :href="blobInfo.permalinkPath"
           :class="$options.buttonClassList"
           class="js-data-file-blob-permalink-url"
    +      @click.prevent="moveToFilePermalink"
         >
           {{ $options.i18n.permalink }}
         </gl-button>
    

    I'm not entirely sure this doesn't break anything, though. This Vue/HAML duality of this view is kind of weird. WDYT?

NOTE: ShortcutsBlob is currently used in both Vue and HAML version of a blob. Rewriting it right now would mean two different implementation of the same functionality. This should be done once we rely only on the Vue version of the blob. The scenarios in which we fallback to HAML:

  • Highlight.js does not support the language, or if an error occurred during highlighting.
  • The file type is listed in our legacy file types (mostly dependency files, this epic will get rid of remaining dependency fallbacks)