From f32661ad78d1fa0334fecf9516e4c9a2ca7793ab Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Fri, 14 May 2021 16:00:54 +0200
Subject: [PATCH] Rework left sidebar * Move Buttons from left sidebar to
 JSTree * Use light style for collapse sidebar buttons

---
 app/assets/javascripts/editor/editor.js.erb   | 12 +++---
 app/assets/javascripts/editor/submissions.js  |  2 +-
 app/assets/stylesheets/base.css.scss          |  2 +-
 app/assets/stylesheets/editor.css.scss        |  4 +-
 .../code_ocean/files_controller.rb            |  2 +-
 app/views/exercises/_editor.html.slim         | 24 +++++++++--
 app/views/exercises/_editor_button.html.slim  |  2 +-
 .../exercises/_editor_file_tree.html.slim     | 42 +++++++++----------
 app/views/exercises/_editor_frame.html.slim   |  4 +-
 app/views/exercises/_editor_output.html.slim  |  4 +-
 lib/file_tree.rb                              | 15 +++----
 spec/lib/file_tree_spec.rb                    | 24 ++++++++---
 12 files changed, 80 insertions(+), 57 deletions(-)

diff --git a/app/assets/javascripts/editor/editor.js.erb b/app/assets/javascripts/editor/editor.js.erb
index 87140329..f94f9e16 100644
--- a/app/assets/javascripts/editor/editor.js.erb
+++ b/app/assets/javascripts/editor/editor.js.erb
@@ -198,7 +198,7 @@ var CodeOceanEditor = {
     },
 
     hideSpinner: function () {
-        $('button i.fa').show();
+        $('button i.fa, button i.far, button i.fas').show();
         $('button i.fa-spin').hide();
     },
 
@@ -212,7 +212,7 @@ var CodeOceanEditor = {
 
     resizeParentOfAceEditor: function (element) {
         // calculate needed size: window height - position of top of ACE editor - height of autosave label below editor - 5 for bar margins
-        var windowHeight = window.innerHeight - $(element).offset().top - $('#autosave-label').height() - 5;
+        var windowHeight = window.innerHeight - $(element).offset().top - $('#statusbar').height() - 5;
         $(element).parent().height(windowHeight);
     },
 
@@ -245,7 +245,7 @@ var CodeOceanEditor = {
             document.insertLines(0, content.text().split(/\n/));
             // remove last (empty) that is there by default line
             document.removeLines(document.getLength() - 1, document.getLength() - 1);
-            editor.setReadOnly($(element).data('read-only') !== undefined);
+            editor.setReadOnly($(element).parent().data('read-only') !== undefined);
             if (editor.getReadOnly()) {
                 editor.setHighlightActiveLine(false);
                 editor.setHighlightGutterLine(false);
@@ -341,11 +341,9 @@ var CodeOceanEditor = {
 
     initializeFileTreeButtons: function () {
         $('#create-file').on('click', this.showFileDialog.bind(this));
-        $('#create-file-collapsed').on('click', this.showFileDialog.bind(this));
         $('#destroy-file').on('click', this.confirmDestroy.bind(this));
         $('#destroy-file-collapsed').on('click', this.confirmDestroy.bind(this));
         $('#download').on('click', this.downloadCode.bind(this));
-        $('#download-collapsed').on('click', this.downloadCode.bind(this));
         $('#request-for-comments').on('click', this.requestComments.bind(this));
     },
 
@@ -580,7 +578,7 @@ var CodeOceanEditor = {
 
     toggleButtonStates: function () {
         $('#destroy-file').prop('disabled', this.active_frame.data('role') !== 'user_defined_file');
-        $('#start-over-active-file').prop('disabled', this.active_frame.data('role') === 'user_defined_file');
+        $('#start-over-active-file').prop('disabled', this.active_frame.data('role') === 'user_defined_file' || this.active_frame.data('read-only') !== undefined);
         $('#dummy').toggle(!this.fileActionsAvailable());
         $('#render').toggle(this.isActiveFileRenderable());
         $('#run').toggle(this.isActiveFileRunnable() && !this.running);
@@ -654,7 +652,7 @@ var CodeOceanEditor = {
     },
 
     showSpinner: function (initiator) {
-        $(initiator).find('i.fa').hide();
+        $(initiator).find('i.fa, i.far, i.fas').hide();
         $(initiator).find('i.fa-spin').show();
     },
 
diff --git a/app/assets/javascripts/editor/submissions.js b/app/assets/javascripts/editor/submissions.js
index 05064bc0..af7c78bb 100644
--- a/app/assets/javascripts/editor/submissions.js
+++ b/app/assets/javascripts/editor/submissions.js
@@ -3,7 +3,7 @@ CodeOceanEditorSubmissions = {
 
   AUTOSAVE_INTERVAL: 15 * 1000,
   autosaveTimer: null,
-  autosaveLabel: "#autosave-label span",
+  autosaveLabel: "#statusbar span",
 
   /**
    * Submission-Creation
diff --git a/app/assets/stylesheets/base.css.scss b/app/assets/stylesheets/base.css.scss
index 1a7598dd..a4b1d82a 100644
--- a/app/assets/stylesheets/base.css.scss
+++ b/app/assets/stylesheets/base.css.scss
@@ -12,7 +12,7 @@ h1, h2, h3, h4, h5, h6 {
   color: rgba(70, 70, 70, 1);
 }
 
-i.fa {
+i.fa, i.far, i.fas {
   margin-right: 0.5em;
 }
 
diff --git a/app/assets/stylesheets/editor.css.scss b/app/assets/stylesheets/editor.css.scss
index 69eb7db5..c9d4aeb1 100644
--- a/app/assets/stylesheets/editor.css.scss
+++ b/app/assets/stylesheets/editor.css.scss
@@ -85,10 +85,10 @@ button i.fa-spin {
   display: none;
 }
 
-#autosave-label{
+#statusbar{
   visibility: hidden;
+  margin-top: .2em;
   height: 1.6em;
-  text-align:  right;
   color: #777;
   font-size: 0.8em;
 }
diff --git a/app/controllers/code_ocean/files_controller.rb b/app/controllers/code_ocean/files_controller.rb
index 21a31664..db268a79 100644
--- a/app/controllers/code_ocean/files_controller.rb
+++ b/app/controllers/code_ocean/files_controller.rb
@@ -18,7 +18,7 @@ module CodeOcean
         @file.content = content
       end
       authorize!
-      create_and_respond(object: @file, path: proc { implement_exercise_path(@file.context.exercise, tab: 2) })
+      create_and_respond(object: @file, path: proc { implement_exercise_path(@file.context.exercise) })
     end
 
     def create_and_respond(options = {})
diff --git a/app/views/exercises/_editor.html.slim b/app/views/exercises/_editor.html.slim
index a17a91ac..4519a0df 100644
--- a/app/views/exercises/_editor.html.slim
+++ b/app/views/exercises/_editor.html.slim
@@ -24,13 +24,29 @@
       = render('editor_button', data: {:'data-placement' => 'top', :'data-toggle' => 'tooltip', :'data-container' => 'body'}, icon: 'fa fa-trophy', id: 'assess', label: t('exercises.editor.score'), title: t('shared.tooltips.shortcut', shortcut: 'ALT + s')) unless @embed_options[:disable_score]
       - unless hide_rfc_button
         = render('editor_button', icon: 'fa fa-comment', id: 'requestComments', label: t('exercises.editor.requestComments'), title: t('exercises.editor.requestCommentsTooltip'))
+
     - @files.each do |file|
       - file.read_only = true if @embed_options[:read_only]
       = render('editor_frame', exercise: exercise, file: file)
-    #autosave-label
-      = t('exercises.editor.lastsaved')
-      span
-      button style="display:none" id="autosave"
+
+    #statusbar.d-flex.justify-content-between
+      div
+        - if !@embed_options[:disable_download] && @exercise.hide_file_tree?
+          button#download.p-0.border-0.btn-link.visible.bg-white
+            i.fas.fa-arrow-down
+            = t('exercises.editor.download')
+
+      div
+        = t('exercises.editor.lastsaved')
+        span
+        button style="display:none" id="autosave"
+
+        = " | "
+
+        button#start-over-active-file.p-0.border-0.btn-link.bg-white data-message-confirm=t('exercises.editor.confirm_start_over_active_file') data-url=reload_exercise_path(@exercise)
+          i.fa.fa-history
+          = t('exercises.editor.start_over_active_file')
+
   - unless @embed_options[:disable_run] && @embed_options[:disable_score]
     div id='output_sidebar' class='output-col-collapsed' = render('exercises/editor_output', external_user_id: external_user_id, consumer_id: consumer_id )
 
diff --git a/app/views/exercises/_editor_button.html.slim b/app/views/exercises/_editor_button.html.slim
index 3c14f0b2..98240c08 100644
--- a/app/views/exercises/_editor_button.html.slim
+++ b/app/views/exercises/_editor_button.html.slim
@@ -1,4 +1,4 @@
 button.btn class=local_assigns.fetch(:classes, 'btn-primary') *local_assigns.fetch(:data, {}) disabled=local_assigns.fetch(:disabled, false) id=id title=local_assigns[:title] type='button'
-  i.fa.fa-circle-o-notch.fa-spin
+  i.fa.fa-circle-o-notch.fa-spin class=(label.blank? ? "m-0" : '')
   i class=(label.present? ? icon : "#{icon} m-0")
   = label
diff --git a/app/views/exercises/_editor_file_tree.html.slim b/app/views/exercises/_editor_file_tree.html.slim
index f41f35aa..7f7d28b4 100644
--- a/app/views/exercises/_editor_file_tree.html.slim
+++ b/app/views/exercises/_editor_file_tree.html.slim
@@ -1,44 +1,42 @@
 div id='sidebar-collapsed' class=(@exercise.hide_file_tree && @tips.blank? ? '' : 'd-none')
-  = render('editor_button', classes: 'btn-block btn-primary btn', data: {:'data-toggle' => 'tooltip', :'data-placement' => 'right'}, icon: 'fa fa-plus-square', id: 'sidebar-collapse-collapsed', label:'', title:t('exercises.editor.expand_action_sidebar'))
-
-  - if @exercise.allow_file_creation and not @exercise.hide_file_tree?
-    = render('editor_button', classes: 'btn-block btn-primary btn enforce-top-margin', data: {:'data-cause' => 'file', :'data-toggle' => 'tooltip', :'data-placement' => 'right'}, icon: 'fa fa-plus', id: 'create-file-collapsed', label:'', title: t('exercises.editor.create_file'))
+  = render('editor_button', classes: 'btn-block btn-outline-dark btn', data: {:'data-toggle' => 'tooltip', :'data-placement' => 'right'}, icon: 'fa fa-plus-square', id: 'sidebar-collapse-collapsed', label:'', title:t('exercises.editor.expand_action_sidebar'))
 
   - unless @embed_options[:disable_hints] or @tips.blank?
     = render('editor_button', classes: 'btn-block btn-secondary btn mb-4', data: {:'data-toggle' => 'tooltip', :'data-placement' => 'right'}, icon: 'fa fa-lightbulb', id: 'tips-collapsed', label:'', title: t('exercises.form.tips'))
 
-  - unless @embed_options[:disable_download]
-    = render('editor_button', classes: 'btn-block btn-primary btn enforce-top-margin', data: {:'data-toggle' => 'tooltip', :'data-placement' => 'right'}, icon: 'fa fa-download', id: 'download-collapsed', label:'', title: t('exercises.editor.download'))
-  = render('editor_button', classes: "btn-block btn-outline-warning btn enforce-top-margin #{@exercise.hide_file_tree || files.count < 2 && !@exercise.allow_file_creation ? 'd-none' : ''}", data: {:'data-message-confirm' => t('exercises.editor.confirm_start_over_active_file'), :'data-url' => reload_exercise_path(@exercise), :'data-toggle' => 'tooltip', :'data-placement' => 'right'}, icon: 'fa fa-history', id: 'start-over-active-file-collapsed', label: '', title: t('exercises.editor.start_over_active_file'))
   //- if !@course_token.blank?
     = render('editor_button', classes: 'btn-block btn-primary btn enforce-top-margin', data: {:'data-toggle' => 'tooltip', :'data-placement' => 'right'}, icon: 'fa fa-search', id: 'sidebar-search-collapsed', label: '', title: t('search.search_in_forum'))
-  = render('editor_button', classes: 'btn-block btn-outline-danger btn enforce-top-margin', data: {:'data-message-confirm' => t('exercises.editor.confirm_start_over'), :'data-url' => reload_exercise_path(@exercise), :'data-toggle' => 'tooltip', :'data-placement' => 'right'}, icon: 'fa fa-history', id: 'start-over-collapsed', label:'', title: t('exercises.editor.start_over'))
 
 div.h-100.col-sm-12.enforce-bottom-margin id='sidebar-uncollapsed' class=(@exercise.hide_file_tree && @tips.blank? ? 'd-none' : '')
   .position-absolute.d-flex.mb-1.w-100 style="overflow: auto; left: 0; top: 0; height: 100%;"
     .w-100
-      = render('editor_button', classes: 'btn-block btn-primary btn', icon: 'fa fa-minus-square', id: 'sidebar-collapse', label: t('exercises.editor.collapse_action_sidebar'))
+      = render('editor_button', classes: 'btn-block btn-outline-dark btn', icon: 'fa fa-minus-square', id: 'sidebar-collapse', label: t('exercises.editor.collapse_action_sidebar'))
 
-      div class=(@exercise.hide_file_tree ? 'd-none' : '')
-        hr
+      - unless @exercise.hide_file_tree
+        div
+          hr
 
-        #files data-entries=FileTree.new(files).to_js_tree
+          .card.border-secondary
+            .card-header.d-flex.justify-content-between.align-items-center.px-0.py-1
+              .px-2 = I18n.t('exercises.editor_file_tree.file_root')
+              div
+                - if @exercise.allow_file_creation
+                  = render('editor_button', classes: 'btn-default btn-sm', data: {:'data-toggle' => 'tooltip', :'data-cause' => 'file'}, icon: 'fa fa-plus', id: 'create-file', label: '', title: t('exercises.editor.create_file'))
+                  = render('editor_button', classes: 'btn-default btn-sm', data: {:'data-toggle' => 'tooltip', :'data-cause' => 'file', :'data-message-confirm' => t('shared.confirm_destroy') }, icon: 'far fa-trash-alt', id: 'destroy-file', label: '', title: t('exercises.editor.destroy_file'))
+                - unless @embed_options[:disable_download]
+                  = render('editor_button', classes: 'btn-default btn-sm', data: {:'data-toggle' => 'tooltip'}, icon: 'fas fa-arrow-down', id: 'download', label:'', title: t('exercises.editor.download'))
+                = render('editor_button', classes: 'btn-default btn-sm', data: {:'data-toggle' => 'tooltip', :'data-message-confirm' => t('exercises.editor.confirm_start_over'), :'data-url' => reload_exercise_path(@exercise)}, icon: 'fa fa-history', id: 'start-over', label: '', title: t('exercises.editor.start_over'))
 
-        hr
+            .card-body.pt-0.pr-0.pl-1.pb-1
 
-      - if @exercise.allow_file_creation and not @exercise.hide_file_tree?
-        = render('editor_button', classes: 'btn-block btn-primary btn', data: {:'data-cause' => 'file'}, icon: 'fa fa-plus', id: 'create-file', label: t('exercises.editor.create_file'))
-        = render('editor_button', classes: 'btn-block btn-warning btn', data: {:'data-cause' => 'file', :'data-message-confirm' => t('shared.confirm_destroy')}, icon: 'fa fa-times', id: 'destroy-file', label: t('exercises.editor.destroy_file'))
+              #files data-entries=FileTree.new(files).to_js_tree
+
+          hr
 
       - unless @embed_options[:disable_hints] or @tips.blank?
         = render(partial: 'tips_content')
         .mb-4
 
-      - unless @embed_options[:disable_download]
-        = render('editor_button', classes: 'btn-block btn-primary btn enforce-top-margin', icon: 'fa fa-download', id: 'download', label: t('exercises.editor.download'))
-      = render('editor_button', classes: "btn-block btn-outline-warning btn #{@exercise.hide_file_tree || files.count < 2 && !@exercise.allow_file_creation ? 'd-none' : ''}", data: {:'data-message-confirm' => t('exercises.editor.confirm_start_over_active_file'), :'data-url' => reload_exercise_path(@exercise)}, icon: 'fa fa-history', id: 'start-over-active-file', label: t('exercises.editor.start_over_active_file'))
-      = render('editor_button', classes: 'btn-block btn-outline-danger btn', data: {:'data-message-confirm' => t('exercises.editor.confirm_start_over'), :'data-url' => reload_exercise_path(@exercise)}, icon: 'fa fa-history', id: 'start-over', label: t('exercises.editor.start_over'))
-
       //- if !@course_token.blank?
           .input-group.enforce-top-margin
             .enforce-right-margin
@@ -48,4 +46,4 @@ div.h-100.col-sm-12.enforce-bottom-margin id='sidebar-uncollapsed' class=(@exerc
                 i.fa.fa-search
 
 - if @exercise.allow_file_creation?
-  = render('shared/modal', id: 'modal-file', template: 'code_ocean/files/_form', title: t('exercises.editor.create_file'))
\ No newline at end of file
+  = render('shared/modal', id: 'modal-file', template: 'code_ocean/files/_form', title: t('exercises.editor.create_file'))
diff --git a/app/views/exercises/_editor_frame.html.slim b/app/views/exercises/_editor_frame.html.slim
index 049183eb..2c86fcc8 100644
--- a/app/views/exercises/_editor_frame.html.slim
+++ b/app/views/exercises/_editor_frame.html.slim
@@ -1,4 +1,4 @@
-.frame data-executable=file.file_type.executable? data-filename=file.name_with_extension data-renderable=file.file_type.renderable? data-role=file.role data-binary=file.file_type.binary? data-context-type=file.context_type
+.frame data-executable=file.file_type.executable? data-filename=file.name_with_extension data-renderable=file.file_type.renderable? data-role=file.role data-binary=file.file_type.binary? data-context-type=file.context_type data-read-only=file.read_only
   - if file.file_type.binary?
     .binary-file data-file-id=file.ancestor_id
       - if file.file_type.renderable?
@@ -12,4 +12,4 @@
         = link_to(file.native_file.file.filename, file.native_file.url)
   - else
     .editor-content.d-none data-file-id=file.ancestor_id = file.content
-    .editor data-file-id=file.ancestor_id data-indent-size=file.file_type.indent_size data-mode=file.file_type.editor_mode data-read-only=file.read_only data-allow-auto-completion=exercise.allow_auto_completion.to_s data-id=file.id
\ No newline at end of file
+    .editor data-file-id=file.ancestor_id data-indent-size=file.file_type.indent_size data-mode=file.file_type.editor_mode data-allow-auto-completion=exercise.allow_auto_completion.to_s data-id=file.id
\ No newline at end of file
diff --git a/app/views/exercises/_editor_output.html.slim b/app/views/exercises/_editor_output.html.slim
index 0803d0b6..d3a7a246 100644
--- a/app/views/exercises/_editor_output.html.slim
+++ b/app/views/exercises/_editor_output.html.slim
@@ -1,8 +1,8 @@
 div id='output_sidebar_collapsed'
-  = render('editor_button', classes: 'btn-block btn-primary btn', data: {:'data-toggle' => 'tooltip', :'data-placement' => 'left'}, title: t('exercises.editor.expand_output_sidebar'), icon: 'fa fa-plus-square', id: 'toggle-sidebar-output-collapsed', label: '')
+  = render('editor_button', classes: 'btn-block btn-outline-dark btn', data: {:'data-toggle' => 'tooltip', :'data-placement' => 'left'}, title: t('exercises.editor.expand_output_sidebar'), icon: 'fa fa-plus-square', id: 'toggle-sidebar-output-collapsed', label: '')
 div.h-100 id='output_sidebar_uncollapsed' class='d-none col-sm-12 enforce-bottom-margin' data-message-no-output=t('exercises.implement.no_output_yet')
   .row
-    = render('editor_button', classes: 'btn-block btn-primary btn', icon: 'fa fa-minus-square', id: 'toggle-sidebar-output', label: t('exercises.editor.collapse_output_sidebar'))
+    = render('editor_button', classes: 'btn-block btn-outline-dark btn', icon: 'fa fa-minus-square', id: 'toggle-sidebar-output', label: t('exercises.editor.collapse_output_sidebar'))
 
   div.position-absolute.d-flex.mb-1.w-100 style="overflow: auto; left: 0; bottom: 0; height: calc(100% - 3rem);"
     div.w-100
diff --git a/lib/file_tree.rb b/lib/file_tree.rb
index bfcf766a..dd1fe2ac 100644
--- a/lib/file_tree.rb
+++ b/lib/file_tree.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-class FileTree < Tree::TreeNode
+class FileTree
   def file_icon(file)
     if file.file_type.audio?
       'fa fa-file-audio-o'
@@ -26,9 +26,11 @@ class FileTree < Tree::TreeNode
   private :folder_icon
 
   def initialize(files = [])
-    super(root_label)
+    # Our tree needs a root node, but we won't display it.
+    @root = Tree::TreeNode.new('ROOT')
+
     files.uniq(&:name_with_extension).each do |file|
-      parent = self
+      parent = @root
       (file.path || '').split('/').each do |segment|
         node = parent.children.detect {|child| child.name == segment } || parent.add(Tree::TreeNode.new(segment))
         parent = node
@@ -60,15 +62,10 @@ class FileTree < Tree::TreeNode
   end
   private :node_icon
 
-  def root_label
-    I18n.t('exercises.editor_file_tree.file_root')
-  end
-  private :root_label
-
   def to_js_tree
     {
       core: {
-        data: map_to_js_tree(self),
+        data: @root.children.map {|child| map_to_js_tree(child) },
       },
     }.to_json
   end
diff --git a/spec/lib/file_tree_spec.rb b/spec/lib/file_tree_spec.rb
index 8ef6ad7e..fe14d4fa 100644
--- a/spec/lib/file_tree_spec.rb
+++ b/spec/lib/file_tree_spec.rb
@@ -81,16 +81,16 @@ describe FileTree do
 
     it 'creates a root node' do
       # Instead of checking #initialize on the parent, we validate #set_as_root!
-      expect(file_tree).to receive(:set_as_root!).and_call_original
+      expect(Tree::TreeNode).to receive(:new).and_call_original.at_least(:once)
       file_tree.send(:initialize)
     end
 
     it 'creates tree nodes for every file' do
-      expect(file_tree.select(&:content).map(&:content)).to eq(files)
+      expect(file_tree.instance_variable_get(:@root).select(&:content).map(&:content)).to eq(files)
     end
 
     it 'creates tree nodes for intermediary path segments' do
-      expect(file_tree.reject(&:content).reject(&:is_root?).map(&:name)).to eq(files.first.path.split('/'))
+      expect(file_tree.instance_variable_get(:@root).reject(&:content).reject(&:is_root?).map(&:name)).to eq(files.first.path.split('/'))
     end
   end
 
@@ -179,8 +179,22 @@ describe FileTree do
       expect(js_tree).to be_a(String)
     end
 
-    it 'produces the required JSON format' do
-      expect(JSON.parse(js_tree).deep_symbolize_keys).to eq(core: {data: file_tree.send(:map_to_js_tree, file_tree)})
+    context 'without any file' do
+      it 'produces the required JSON format' do
+        expect(JSON.parse(js_tree).deep_symbolize_keys).to eq(core: {data: []})
+      end
+    end
+
+    context 'with files' do
+      let(:files) { FactoryBot.build_list(:file, 10, context: nil, path: 'foo/bar/baz') }
+      let(:file_tree) { described_class.new(files) }
+      let(:js_tree) { file_tree.to_js_tree }
+
+      it 'produces the required JSON format with a file' do
+        # We ignore the root node and only use the children here
+        child_tree = file_tree.send(:map_to_js_tree, file_tree.instance_variable_get(:@root).children.first)
+        expect(JSON.parse(js_tree).deep_symbolize_keys).to eq(core: {data: [child_tree]})
+      end
     end
   end
 end