Skip to content
Snippets Groups Projects

Ace ventura

Merged Connor Shea requested to merge ace-ventura into master
4 unresolved threads

What does this MR do?

Makes Ace only load when it's actually necessary. The only two places it seems to be used is for Snippets and the File (Blob) Editor.

Before minification or compression, this takes the application.js down from 2.2MB to 1.6MB, and also removes two inline scripts :D

Compressed/Minified: master: 317KB/1.1MB, ace-ventura: 220KB/771KB

Are there points in the code the reviewer needs to double check?

That this doesn't just completely break anything.

Why was this MR needed?

Ace is a big library, this allows us to only load it when necessary.

What are the relevant issue numbers?

#14372 (closed)

Does this MR meet the acceptance criteria?

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
1 /*= require_tree . */
2
3 (function() {
4 $(function() {
5 url = $(".js-edit-blob-form").data("relative-url-root");
  • 1 /*= require_tree . */
    2
    3 (function() {
    4 $(function() {
    5 editor = ace.edit("editor")
  • Connor Shea Added 1 commit:

    Added 1 commit:

    • 532a9379 - Don't use global variables.
  • Reassigned to @tauriedavis

  • Jacob Schatz Assignee removed

    Assignee removed

  • Reassigned to @connorshea

  • @connorshea let me know once builds pass.

  • Connor Shea Added 1 commit:

    Added 1 commit:

    • 5cb96c02 - Blob has to be global, unfortunately.
  • Connor Shea Added 1 commit:

    Added 1 commit:

    • c5189783 - Don't use global variables.
  • Connor Shea Added 1 commit:

    Added 1 commit:

    • 415733e2 - Don't use global variables.
  • Sean McGivern Added 59 commits:

    Added 59 commits:

  • 44 44 end
    45 45
    46 46 step 'I should see its content with new lines preserved at end of file' do
    47 expect(evaluate_script('blob.editor.getValue()')).to eq "Sample\n\n\n"
  • Reassigned to @jschatz1

  • Author Contributor

    Builds are passing now :thumbsup: @jschatz1

  • 16 19 = link_to '#preview', 'data-preview-url' => namespace_project_preview_blob_path(@project.namespace, @project, @id) do
    17 20 = editing_preview_title(@blob.name)
    18 21
    19 = form_tag(namespace_project_update_blob_path(@project.namespace, @project, @id), method: :put, class: 'form-horizontal js-quick-submit js-requires-input js-edit-blob-form') do
    22 = form_tag(namespace_project_update_blob_path(@project.namespace, @project, @id), method: :put, class: 'form-horizontal js-quick-submit js-requires-input js-edit-blob-form', data: {'relative-url-root' => "#{Rails.application.config.relative_url_root}", 'assets-prefix' => "#{Gitlab::Application.config.assets.prefix}", 'blob-language' => "#{@blob.language.try(:ace_mode)}" }) do
  • Reassigned to @smcgivern

  • @smcgivern Would love your review on this too.

  • Sean McGivern Added 1 commit:

    Added 1 commit:

    • 9cf6d451 - Move editor paths to helper
  • I tested this locally with precompiled assets in dev mode, and yep, it works great for both editing existing files and creating new ones! Syntax highlighting modes are still applied, and we save the 100-odd KB from our application.js when you aren't editing a file.

    Awesome work @connorshea, and good patience too :wink:

  • Reassigned to @jschatz1

  • Jacob Schatz Enabled an automatic merge when the build for 9cf6d451 succeeds

    Enabled an automatic merge when the build for 9cf6d451 succeeds

  • Jacob Schatz Status changed to merged

    Status changed to merged

  • Jacob Schatz mentioned in commit 8fc800cb

    mentioned in commit 8fc800cb

  • Stan Hu mentioned in merge request !5909 (merged)

    mentioned in merge request !5909 (merged)

  • Stan Hu mentioned in commit 310eef45

    mentioned in commit 310eef45

  • Picked into 8-11-stable, will go into 8.11.0-rc6.

  • Rubén Dávila Removed ~149423 label

    Removed ~149423 label

  • Jacob Schatz mentioned in commit d53ef2e8

    mentioned in commit d53ef2e8

  • Stan Hu mentioned in commit 4030d5ab

    mentioned in commit 4030d5ab

  • Please register or sign in to reply
    Loading