Commit 19740871 authored by Jason Lee's avatar Jason Lee

Avoid render edit_form in each notes.

Use RJS to render edit note feature.

Before:

```
Rendered projects/notes/_note.html.haml (27.9ms)
Rendered projects/_zen.html.haml (0.3ms)
Rendered projects/notes/_hints.html.haml (0.7ms)
Rendered projects/_md_preview.html.haml (3.9ms)
Rendered projects/notes/_edit_form.html.haml (6.9ms)
Rendered projects/notes/_note.html.haml (17.7ms)
Rendered projects/_zen.html.haml (0.3ms)
Rendered projects/notes/_hints.html.haml (0.6ms)
Rendered projects/_md_preview.html.haml (3.4ms)
Rendered projects/notes/_edit_form.html.haml (7.0ms)
```

After:

```
Rendered projects/notes/_note.html.haml (13.8ms)
Rendered projects/notes/_note.html.haml (7.1ms)
Rendered projects/notes/_note.html.haml (9.5ms)
Rendered projects/notes/_note.html.haml (8.5ms)
```

This change reduce at least 6ms * N ('N' - number of notes).
parent ca25289b
...@@ -29,7 +29,6 @@ class @Notes ...@@ -29,7 +29,6 @@ class @Notes
$(document).on "ajax:success", "form.edit_note", @updateNote $(document).on "ajax:success", "form.edit_note", @updateNote
# Edit note link # Edit note link
$(document).on "click", ".js-note-edit", @showEditForm
$(document).on "click", ".note-edit-cancel", @cancelEdit $(document).on "click", ".note-edit-cancel", @cancelEdit
# Reopen and close actions for Issue/MR combined with note form submit # Reopen and close actions for Issue/MR combined with note form submit
...@@ -67,7 +66,6 @@ class @Notes ...@@ -67,7 +66,6 @@ class @Notes
$(document).off "ajax:success", ".js-main-target-form" $(document).off "ajax:success", ".js-main-target-form"
$(document).off "ajax:success", ".js-discussion-note-form" $(document).off "ajax:success", ".js-discussion-note-form"
$(document).off "ajax:success", "form.edit_note" $(document).off "ajax:success", "form.edit_note"
$(document).off "click", ".js-note-edit"
$(document).off "click", ".note-edit-cancel" $(document).off "click", ".note-edit-cancel"
$(document).off "click", ".js-note-delete" $(document).off "click", ".js-note-delete"
$(document).off "click", ".js-note-attachment-delete" $(document).off "click", ".js-note-attachment-delete"
...@@ -287,13 +285,14 @@ class @Notes ...@@ -287,13 +285,14 @@ class @Notes
Adds a hidden div with the original content of the note to fill the edit note form with Adds a hidden div with the original content of the note to fill the edit note form with
if the user cancels if the user cancels
### ###
showEditForm: (e) -> showEditForm: (note, formHTML) ->
e.preventDefault() nodeText = note.find(".note-text");
note = $(this).closest(".note") nodeText.hide()
note.find('.note-edit-form').remove()
nodeText.after(formHTML)
note.find(".note-body > .note-text").hide() note.find(".note-body > .note-text").hide()
note.find(".note-header").hide() note.find(".note-header").hide()
base_form = note.find(".note-edit-form") form = note.find(".note-edit-form")
form = base_form.clone().insertAfter(base_form)
form.addClass('current-note-edit-form gfm-form') form.addClass('current-note-edit-form gfm-form')
form.find('.div-dropzone').remove() form.find('.div-dropzone').remove()
......
...@@ -3,7 +3,7 @@ class Projects::NotesController < Projects::ApplicationController ...@@ -3,7 +3,7 @@ class Projects::NotesController < Projects::ApplicationController
before_action :authorize_read_note! before_action :authorize_read_note!
before_action :authorize_create_note!, only: [:create] before_action :authorize_create_note!, only: [:create]
before_action :authorize_admin_note!, only: [:update, :destroy] before_action :authorize_admin_note!, only: [:update, :destroy]
before_action :find_current_user_notes, except: [:destroy, :delete_attachment] before_action :find_current_user_notes, except: [:destroy, :edit, :delete_attachment]
def index def index
current_fetched_at = Time.now.to_i current_fetched_at = Time.now.to_i
...@@ -29,6 +29,11 @@ class Projects::NotesController < Projects::ApplicationController ...@@ -29,6 +29,11 @@ class Projects::NotesController < Projects::ApplicationController
end end
end end
def edit
@note = note
render layout: false
end
def update def update
@note = Notes::UpdateService.new(project, current_user, note_params).execute(note) @note = Notes::UpdateService.new(project, current_user, note_params).execute(note)
......
...@@ -7,7 +7,7 @@ ...@@ -7,7 +7,7 @@
.note-header .note-header
- if note_editable?(note) - if note_editable?(note)
.note-actions .note-actions
= link_to '#', title: 'Edit comment', class: 'js-note-edit' do = link_to edit_namespace_project_note_path(note.project.namespace, note.project, note), title: 'Edit comment', remote: true, class: 'js-note-edit' do
= icon('pencil-square-o') = icon('pencil-square-o')
= link_to namespace_project_note_path(note.project.namespace, note.project, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'js-note-delete danger' do = link_to namespace_project_note_path(note.project.namespace, note.project, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'js-note-delete danger' do
...@@ -59,9 +59,6 @@ ...@@ -59,9 +59,6 @@
.note-text .note-text
= preserve do = preserve do
= markdown(note.note, {no_header_anchors: true}) = markdown(note.note, {no_header_anchors: true})
- unless note.system?
-# System notes can't be edited
= render 'projects/notes/edit_form', note: note
- if note.attachment.url - if note.attachment.url
.note-attachment .note-attachment
......
...@@ -7,4 +7,4 @@ ...@@ -7,4 +7,4 @@
= render "projects/notes/form", view: params[:view] = render "projects/notes/form", view: params[:view]
:javascript :javascript
new Notes("#{namespace_project_notes_path(namespace_id: @project.namespace, target_id: @noteable.id, target_type: @noteable.class.name.underscore)}", #{@notes.map(&:id).to_json}, #{Time.now.to_i}, "#{params[:view]}") window._notes = new Notes("#{namespace_project_notes_path(namespace_id: @project.namespace, target_id: @noteable.id, target_type: @noteable.class.name.underscore)}", #{@notes.map(&:id).to_json}, #{Time.now.to_i}, "#{params[:view]}")
$note = $('.note-row-<%= @note.id %>:visible');
_notes.showEditForm($note, '<%= escape_javascript(render('edit_form', note: @note)) %>');
...@@ -658,7 +658,7 @@ Gitlab::Application.routes.draw do ...@@ -658,7 +658,7 @@ Gitlab::Application.routes.draw do
end end
end end
resources :notes, only: [:index, :create, :destroy, :update], constraints: { id: /\d+/ } do resources :notes, constraints: { id: /\d+/ } do
member do member do
delete :delete_attachment delete :delete_attachment
end end
......
...@@ -219,7 +219,7 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps ...@@ -219,7 +219,7 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps
end end
step 'The code block should be unchanged' do step 'The code block should be unchanged' do
expect(page).to have_content("```\nCommand [1]: /usr/local/bin/git , see [text](doc/text)\n```") expect(page).to have_content("Command [1]: /usr/local/bin/git , see [text](doc/text)")
end end
step 'project \'Shop\' has issue \'Bugfix1\' with description: \'Description for issue1\'' do step 'project \'Shop\' has issue \'Bugfix1\' with description: \'Description for issue1\'' do
......
...@@ -65,12 +65,6 @@ describe 'Comments', feature: true do ...@@ -65,12 +65,6 @@ describe 'Comments', feature: true do
end end
describe 'when editing a note', js: true do describe 'when editing a note', js: true do
it 'should contain the hidden edit form' do
page.within("#note_#{note.id}") do
is_expected.to have_css('.note-edit-form', visible: false)
end
end
describe 'editing the note' do describe 'editing the note' do
before do before do
find('.note').hover find('.note').hover
......
...@@ -51,7 +51,6 @@ feature 'Task Lists', feature: true do ...@@ -51,7 +51,6 @@ feature 'Task Lists', feature: true do
expect(page).to have_selector(container) expect(page).to have_selector(container)
expect(page).to have_selector("#{container} .wiki .task-list .task-list-item .task-list-item-checkbox") expect(page).to have_selector("#{container} .wiki .task-list .task-list-item .task-list-item-checkbox")
expect(page).to have_selector("#{container} .js-task-list-field")
expect(page).to have_selector('form.js-issuable-update') expect(page).to have_selector('form.js-issuable-update')
expect(page).to have_selector('a.btn-close') expect(page).to have_selector('a.btn-close')
end end
...@@ -90,7 +89,6 @@ feature 'Task Lists', feature: true do ...@@ -90,7 +89,6 @@ feature 'Task Lists', feature: true do
expect(page).to have_selector('.note .js-task-list-container') expect(page).to have_selector('.note .js-task-list-container')
expect(page).to have_selector('.note .js-task-list-container .task-list .task-list-item .task-list-item-checkbox') expect(page).to have_selector('.note .js-task-list-container .task-list .task-list-item .task-list-item-checkbox')
expect(page).to have_selector('.note .js-task-list-container .js-task-list-field')
end end
it 'is only editable by author' do it 'is only editable by author' do
...@@ -127,7 +125,6 @@ feature 'Task Lists', feature: true do ...@@ -127,7 +125,6 @@ feature 'Task Lists', feature: true do
expect(page).to have_selector(container) expect(page).to have_selector(container)
expect(page).to have_selector("#{container} .wiki .task-list .task-list-item .task-list-item-checkbox") expect(page).to have_selector("#{container} .wiki .task-list .task-list-item .task-list-item-checkbox")
expect(page).to have_selector("#{container} .js-task-list-field")
expect(page).to have_selector('form.js-issuable-update') expect(page).to have_selector('form.js-issuable-update')
expect(page).to have_selector('a.btn-close') expect(page).to have_selector('a.btn-close')
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment