Merge branch '383803-web-ide-editor-font' into 'master'

Add JetBrains Mono as WebIDE editor font

See merge request !109188



Merged-by: Alexandru Croitor <acroitor@gitlab.com>
Approved-by: Stanislav Lashmanov <slashmanov@gitlab.com>
Approved-by: Julia Miocene <jmiocene@gitlab.com>
Approved-by: Alexandru Croitor <acroitor@gitlab.com>
Reviewed-by: Kassio Borges <kborges@gitlab.com>
Co-authored-by: Tomas Vik <tvik@gitlab.com>

.rubocop_todo/rails/helper_instance_variable.yml

@@ -24,7 +24,6 @@ Rails/HelperInstanceVariable:
     - 'app/helpers/graph_helper.rb'
     - 'app/helpers/groups_helper.rb'
     - 'app/helpers/icons_helper.rb'
-    - 'app/helpers/ide_helper.rb'
     - 'app/helpers/import_helper.rb'
     - 'app/helpers/issuables_description_templates_helper.rb'
     - 'app/helpers/issuables_helper.rb'

app/assets/javascripts/ide/init_gitlab_web_ide.js

@@ -38,6 +38,9 @@ export const initGitlabWebIDE = async (el) => {
     filePath,
     mergeRequest: mrId,
     forkInfo: forkInfoJSON,
+    editorFontSrcUrl,
+    editorFontFormat,
+    editorFontFamily,
   } = el.dataset;
 
   const rootEl = setupRootElement(el);
@@ -64,6 +67,11 @@ export const initGitlabWebIDE = async (el) => {
       feedbackIssue: GITLAB_WEB_IDE_FEEDBACK_ISSUE,
       userPreferences: el.dataset.userPreferencesPath,
     },
+    editorFont: {
+      srcUrl: editorFontSrcUrl,
+      fontFamily: editorFontFamily,
+      format: editorFontFormat,
+    },
     async handleStartRemote({ remoteHost, remotePath, connectionToken }) {
       const confirmed = await confirmAction(
         __('Are you sure you want to leave the Web IDE? All unsaved changes will be lost.'),

app/helpers/ide_helper.rb

 # frozen_string_literal: true
 
 module IdeHelper
-  def ide_data
+  def ide_data(project:, branch:, path:, merge_request:, fork_info:)
     {
       'can-use-new-web-ide' => can_use_new_web_ide?.to_s,
       'use-new-web-ide' => use_new_web_ide?.to_s,
       'new-web-ide-help-page-path' => help_page_path('user/project/web_ide/index.md', anchor: 'vscode-reimplementation'),
       'user-preferences-path' => profile_preferences_path,
-      'branch-name' => @branch,
-      'file-path' => @path,
-      'fork-info' => @fork_info&.to_json,
-      'merge-request' => @merge_request
-    }.merge(use_new_web_ide? ? new_ide_data : legacy_ide_data)
+      'branch-name' => branch,
+      'file-path' => path,
+      'fork-info' => fork_info&.to_json,
+      'editor-font-src-url' => font_url('jetbrains-mono/JetBrainsMono.woff2'),
+      'editor-font-family' => 'JetBrains Mono',
+      'editor-font-format' => 'woff2',
+      'merge-request' => merge_request
+    }.merge(use_new_web_ide? ? new_ide_data(project: project) : legacy_ide_data(project: project))
   end
 
   def can_use_new_web_ide?
@@ -24,16 +27,16 @@ def use_new_web_ide?
 
   private
 
-  def new_ide_data
+  def new_ide_data(project:)
     {
-      'project-path' => @project&.path_with_namespace,
+      'project-path' => project&.path_with_namespace,
       'csp-nonce' => content_security_policy_nonce,
       # We will replace these placeholders in the FE
       'ide-remote-path' => ide_remote_path(remote_host: ':remote_host', remote_path: ':remote_path')
     }
   end
 
-  def legacy_ide_data
+  def legacy_ide_data(project:)
     {
       'empty-state-svg-path' => image_path('illustrations/multi_file_editor_empty.svg'),
       'no-changes-state-svg-path' => image_path('illustrations/multi-editor_no_changes_empty.svg'),
@@ -44,10 +47,10 @@ def legacy_ide_data
       'ci-help-page-path' => help_page_path('ci/quick_start/index'),
       'web-ide-help-page-path' => help_page_path('user/project/web_ide/index.md'),
       'render-whitespace-in-code': current_user.render_whitespace_in_code.to_s,
-      'default-branch' => @project && @project.default_branch,
-      'project' => convert_to_project_entity_json(@project),
-      'enable-environments-guidance' => enable_environments_guidance?.to_s,
-      'preview-markdown-path' => @project && preview_markdown_path(@project),
+      'default-branch' => project && project.default_branch,
+      'project' => convert_to_project_entity_json(project),
+      'enable-environments-guidance' => enable_environments_guidance?(project).to_s,
+      'preview-markdown-path' => project && preview_markdown_path(project),
       'web-terminal-svg-path' => image_path('illustrations/web-ide_promotion.svg'),
       'web-terminal-help-path' => help_page_path('user/project/web_ide/index.md', anchor: 'interactive-web-terminals-for-the-web-ide'),
       'web-terminal-config-help-path' => help_page_path('user/project/web_ide/index.md', anchor: 'web-ide-configuration-file'),
@@ -61,8 +64,8 @@ def convert_to_project_entity_json(project)
     API::Entities::Project.represent(project, current_user: current_user).to_json
   end
 
-  def enable_environments_guidance?
-    experiment(:in_product_guidance_environments_webide, project: @project) do |e|
+  def enable_environments_guidance?(project)
+    experiment(:in_product_guidance_environments_webide, project: project) do |e|
       e.candidate { !has_dismissed_ide_environments_callout? }
 
       e.run

app/views/ide/_show.html.haml

@@ -7,4 +7,4 @@
   - content_for :prefetch_asset_tags do
     - webpack_preload_asset_tag('monaco')
 
-= render partial: 'shared/ide_root', locals: { data: ide_data, loading_text: _('Loading the GitLab IDE...') }
+= render partial: 'shared/ide_root', locals: { data: ide_data(project: @project, branch: @branch, path: @path, merge_request: @merge_request, fork_info: @fork_info), loading_text: _('Loading the GitLab IDE...') }

spec/frontend/ide/init_gitlab_web_ide_spec.js

@@ -32,6 +32,9 @@ const TEST_START_REMOTE_PARAMS = {
   remotePath: '/test/projects/f oo',
   connectionToken: '123abc',
 };
+const TEST_EDITOR_FONT_SRC_URL = 'http://gitlab.test/assets/jetbrains-mono/JetBrainsMono.woff2';
+const TEST_EDITOR_FONT_FORMAT = 'woff2';
+const TEST_EDITOR_FONT_FAMILY = 'JebBrains Mono';
 
 describe('ide/init_gitlab_web_ide', () => {
   let resolveConfirm;
@@ -49,6 +52,9 @@ describe('ide/init_gitlab_web_ide', () => {
     el.dataset.userPreferencesPath = TEST_USER_PREFERENCES_PATH;
     el.dataset.mergeRequest = TEST_MR_ID;
     el.dataset.filePath = TEST_FILE_PATH;
+    el.dataset.editorFontSrcUrl = TEST_EDITOR_FONT_SRC_URL;
+    el.dataset.editorFontFormat = TEST_EDITOR_FONT_FORMAT;
+    el.dataset.editorFontFamily = TEST_EDITOR_FONT_FAMILY;
 
     document.body.append(el);
   };
@@ -103,6 +109,11 @@ describe('ide/init_gitlab_web_ide', () => {
           userPreferences: TEST_USER_PREFERENCES_PATH,
           feedbackIssue: GITLAB_WEB_IDE_FEEDBACK_ISSUE,
         },
+        editorFont: {
+          srcUrl: TEST_EDITOR_FONT_SRC_URL,
+          fontFamily: TEST_EDITOR_FONT_FAMILY,
+          format: TEST_EDITOR_FONT_FORMAT,
+        },
         handleStartRemote: expect.any(Function),
       });
     });

spec/helpers/ide_helper_spec.rb

@@ -15,16 +15,12 @@
     context 'with vscode_web_ide=true and instance vars set' do
       before do
         stub_feature_flags(vscode_web_ide: true)
-
-        self.instance_variable_set(:@branch, 'master')
-        self.instance_variable_set(:@project, project)
-        self.instance_variable_set(:@path, 'foo/README.md')
-        self.instance_variable_set(:@merge_request, '7')
       end
 
       it 'returns hash' do
-        expect(helper.ide_data)
-          .to eq(
+        expect(helper.ide_data(project: project, branch: 'master', path: 'foo/README.md', merge_request: '7',
+fork_info: nil))
+          .to match(
             'can-use-new-web-ide' => 'true',
             'use-new-web-ide' => 'true',
             'user-preferences-path' => profile_preferences_path,
@@ -35,6 +31,9 @@
             'csp-nonce' => 'test-csp-nonce',
             'ide-remote-path' => ide_remote_path(remote_host: ':remote_host', remote_path: ':remote_path'),
             'file-path' => 'foo/README.md',
+            'editor-font-family' => 'JetBrains Mono',
+            'editor-font-format' => 'woff2',
+            'editor-font-src-url' => a_string_matching(%r{jetbrains-mono/JetBrainsMono}),
             'merge-request' => '7',
             'fork-info' => nil
           )
@@ -43,7 +42,8 @@
       it 'does not use new web ide if user.use_legacy_web_ide' do
         allow(user).to receive(:use_legacy_web_ide).and_return(true)
 
-        expect(helper.ide_data).to include('use-new-web-ide' => 'false')
+        expect(helper.ide_data(project: project, branch: nil, path: nil, merge_request: nil,
+fork_info: nil)).to include('use-new-web-ide' => 'false')
       end
     end
 
@@ -52,9 +52,9 @@
         stub_feature_flags(vscode_web_ide: false)
       end
 
-      context 'when instance vars are not set' do
+      context 'when instance vars and parameters are not set' do
         it 'returns instance data in the hash as nil' do
-          expect(helper.ide_data)
+          expect(helper.ide_data(project: nil, branch: nil, path: nil, merge_request: nil, fork_info: nil))
             .to include(
               'can-use-new-web-ide' => 'false',
               'use-new-web-ide' => 'false',
@@ -73,15 +73,10 @@
         it 'returns instance data in the hash' do
           fork_info = { ide_path: '/test/ide/path' }
 
-          self.instance_variable_set(:@branch, 'master')
-          self.instance_variable_set(:@path, 'foo/bar')
-          self.instance_variable_set(:@merge_request, '1')
-          self.instance_variable_set(:@fork_info, fork_info)
-          self.instance_variable_set(:@project, project)
-
           serialized_project = API::Entities::Project.represent(project, current_user: project.creator).to_json
 
-          expect(helper.ide_data)
+          expect(helper.ide_data(project: project, branch: 'master', path: 'foo/bar', merge_request: '1',
+fork_info: fork_info))
             .to include(
               'branch-name' => 'master',
               'file-path' => 'foo/bar',
@@ -96,12 +91,12 @@
       context 'environments guidance experiment', :experiment do
         before do
           stub_experiments(in_product_guidance_environments_webide: :candidate)
-          self.instance_variable_set(:@project, project)
         end
 
         context 'when project has no enviornments' do
           it 'enables environment guidance' do
-            expect(helper.ide_data).to include('enable-environments-guidance' => 'true')
+            expect(helper.ide_data(project: project, branch: nil, path: nil, merge_request: nil,
+fork_info: nil)).to include('enable-environments-guidance' => 'true')
           end
 
           context 'and the callout has been dismissed' do
@@ -109,7 +104,8 @@
               callout = create(:callout, feature_name: :web_ide_ci_environments_guidance, user: project.creator)
               callout.update!(dismissed_at: Time.now - 1.week)
               allow(helper).to receive(:current_user).and_return(User.find(project.creator.id))
-              expect(helper.ide_data).to include('enable-environments-guidance' => 'false')
+              expect(helper.ide_data(project: project, branch: nil, path: nil, merge_request: nil,
+fork_info: nil)).to include('enable-environments-guidance' => 'false')
             end
           end
         end
@@ -118,7 +114,8 @@
           it 'disables environment guidance' do
             create(:environment, project: project)
 
-            expect(helper.ide_data).to include('enable-environments-guidance' => 'false')
+            expect(helper.ide_data(project: project, branch: nil, path: nil, merge_request: nil,
+fork_info: nil)).to include('enable-environments-guidance' => 'false')
           end
         end
       end