diff --git a/app/components/rapid_diffs/app_component.html.haml b/app/components/rapid_diffs/app_component.html.haml index 83f5ae409494ffbf3477e1d8c36142fad5002857..6f54260f250c121ad6f7d065eafde9528d6b34a3 100644 --- a/app/components/rapid_diffs/app_component.html.haml +++ b/app/components/rapid_diffs/app_component.html.haml @@ -1,4 +1,4 @@ -- if @preload +- if !@lazy - helpers.add_page_startup_api_call @metadata_endpoint - helpers.add_page_startup_api_call @diff_files_endpoint - if @stream_url @@ -23,6 +23,10 @@ = helpers.gl_loading_icon(size: 'sm') .rd-app-content .rd-app-content-header{ data: { hidden_files_warning: true } } + + - if empty_diff? && !@lazy + = render RapidDiffs::EmptyStateComponent.new + .code{ class: helpers.user_color_scheme } %div{ data: { diffs_list: true } } = javascript_tag nonce: content_security_policy_nonce do @@ -30,7 +34,7 @@ requestAnimationFrame(() => { window.performance.mark('rapid-diffs-first-diff-file-shown') }) - if diffs_list? = diffs_list - - else + - elsif !empty_diff? = render RapidDiffs::DiffFileComponent.with_collection(@diffs_slice, parallel_view: @diff_view == :parallel) - if @stream_url #js-stream-container{ data: { diffs_stream_url: @stream_url } } diff --git a/app/components/rapid_diffs/app_component.rb b/app/components/rapid_diffs/app_component.rb index 6e33d11785d853527e96171d2a5b234958fcd79e..039249a772d084ac9f26aa22b7da11635dd9cfc7 100644 --- a/app/components/rapid_diffs/app_component.rb +++ b/app/components/rapid_diffs/app_component.rb @@ -13,7 +13,7 @@ def initialize( update_user_endpoint:, metadata_endpoint:, diff_files_endpoint:, - preload: true + lazy: false ) @diffs_slice = diffs_slice @reload_stream_url = reload_stream_url @@ -23,7 +23,11 @@ def initialize( @update_user_endpoint = update_user_endpoint @metadata_endpoint = metadata_endpoint @diff_files_endpoint = diff_files_endpoint - @preload = preload + @lazy = lazy + end + + def empty_diff? + @diffs_slice.nil? || @diffs_slice.empty? end def browser_visible? diff --git a/app/components/rapid_diffs/empty_state_component.html.haml b/app/components/rapid_diffs/empty_state_component.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..a0128c6891ac984404a40f3ca5451a49a8a27b32 --- /dev/null +++ b/app/components/rapid_diffs/empty_state_component.html.haml @@ -0,0 +1,9 @@ +.rd-app-empty-state.gl-mt-5.gl-text-center + = render Pajamas::CardComponent.new(card_options: { class: "gl-bg-subtle" }) do |c| + - c.with_body do + = render Pajamas::EmptyStateComponent.new(svg_path: 'illustrations/empty-state/empty-commit-md.svg', + title: message) do |c| + + - c.with_description do + -# TODO: Add secondary text + -# TODO: Add call to action button diff --git a/app/components/rapid_diffs/empty_state_component.rb b/app/components/rapid_diffs/empty_state_component.rb new file mode 100644 index 0000000000000000000000000000000000000000..ddb1e48cad4a356c1a8965876d3606c6f34364cb --- /dev/null +++ b/app/components/rapid_diffs/empty_state_component.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module RapidDiffs + class EmptyStateComponent < ViewComponent::Base + def initialize(message: nil) + @message = message || _('There are no changes') + end + + private + + attr_reader :message + end +end diff --git a/app/controllers/concerns/stream_diffs.rb b/app/controllers/concerns/stream_diffs.rb index 8c3a188b5ee21276b051574bf98020021ee6082a..cd1a0260bd8d7eadd6649b917450b34933bf4801 100644 --- a/app/controllers/concerns/stream_diffs.rb +++ b/app/controllers/concerns/stream_diffs.rb @@ -57,11 +57,19 @@ def view def stream_diff_files(options) return unless resource + diffs = resource.diffs_for_streaming(options) + + if params.permit(:offset)[:offset].blank? && diffs.diff_files.empty? + empty_state_component = ::RapidDiffs::EmptyStateComponent.new + response.stream.write empty_state_component.render_in(view_context) + return + end + # NOTE: This is a temporary flag to test out the new diff_blobs if !!ActiveModel::Type::Boolean.new.cast(params.permit(:diff_blobs)[:diff_blobs]) stream_diff_blobs(options) else - resource.diffs_for_streaming(options).diff_files.each do |diff_file| + diffs.diff_files.each do |diff_file| response.stream.write(render_diff_file(diff_file)) end end diff --git a/app/views/projects/merge_requests/creations/rapid_diffs.html.haml b/app/views/projects/merge_requests/creations/rapid_diffs.html.haml index 469ef64265848071d9ef5e09ce08db857b1831f7..cd06311dcbd24f5cadf1fb615fc683a601a9f661 100644 --- a/app/views/projects/merge_requests/creations/rapid_diffs.html.haml +++ b/app/views/projects/merge_requests/creations/rapid_diffs.html.haml @@ -3,5 +3,5 @@ - add_page_specific_style 'page_bundles/merge_request_rapid_diffs' = render "page" do - - args = { diffs_slice: nil, reload_stream_url: @reload_stream_url, stream_url: @stream_url, show_whitespace: @show_whitespace_default, diff_view: diff_view, update_user_endpoint: expose_path(api_v4_user_preferences_path), metadata_endpoint: nil, diff_files_endpoint: @diff_files_endpoint, preload: false } + - args = { diffs_slice: nil, reload_stream_url: @reload_stream_url, stream_url: @stream_url, show_whitespace: @show_whitespace_default, diff_view: diff_view, update_user_endpoint: expose_path(api_v4_user_preferences_path), metadata_endpoint: nil, diff_files_endpoint: @diff_files_endpoint, lazy: true } = render ::RapidDiffs::AppComponent.new(**args) diff --git a/spec/components/rapid_diffs/app_component_spec.rb b/spec/components/rapid_diffs/app_component_spec.rb index aea076136c0e7681a74f493d39f9c1bae4f18268..ad541a351efeefb4458a8948cb0eaf6a43268447 100644 --- a/spec/components/rapid_diffs/app_component_spec.rb +++ b/spec/components/rapid_diffs/app_component_spec.rb @@ -90,7 +90,22 @@ expect(vc_test_controller.view_context.content_for?(:startup_js)).not_to be_nil end - def create_instance + context "when there are no diffs" do + let(:diffs_slice) { [] } + + it "renders empty state component" do + render_component + expect(page).to have_text("There are no changes") + end + + it "does not render empty state when lazy is true" do + instance = create_instance(lazy: true) + render_inline(instance) + expect(page).not_to have_text("There are no changes") + end + end + + def create_instance(lazy: false) described_class.new( diffs_slice:, stream_url:, @@ -99,7 +114,8 @@ def create_instance diff_view:, update_user_endpoint:, metadata_endpoint:, - diff_files_endpoint: + diff_files_endpoint:, + lazy: ) end diff --git a/spec/components/rapid_diffs/empty_state_component_spec.rb b/spec/components/rapid_diffs/empty_state_component_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..e3c1277552528e0fbdd4f070debf908ca1814390 --- /dev/null +++ b/spec/components/rapid_diffs/empty_state_component_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe RapidDiffs::EmptyStateComponent, type: :component, feature_category: :code_review_workflow do + it "renders with default message" do + render_inline(described_class.new) + expect(page).to have_text("There are no changes") + end + + it "renders with custom message" do + custom_message = "No changes found in this merge request" + render_inline(described_class.new(message: custom_message)) + expect(page).to have_text(custom_message) + end +end diff --git a/spec/controllers/concerns/stream_diffs_spec.rb b/spec/controllers/concerns/stream_diffs_spec.rb index 8c29763bce9dc2ba2fe7a0642bb59486653a86f5..6a711357dc1ab7a64f0b408d90cca86f7d778a4f 100644 --- a/spec/controllers/concerns/stream_diffs_spec.rb +++ b/spec/controllers/concerns/stream_diffs_spec.rb @@ -48,4 +48,48 @@ def diff_options expect(controller.new.request.format.to_s).to eq('text/html') end end + + describe '#stream_diff_files' do + let(:controller_instance) { controller.new } + let(:mock_resource) { instance_double(::Commit) } + let(:mock_diffs) { instance_double(Gitlab::Diff::FileCollection::Commit, diff_files: diff_files) } + let(:diff_files) { [] } + let(:response) do + instance_double(ActionDispatch::Response, + stream: instance_double(ActionDispatch::Response::Buffer, write: nil)) + end + + let(:empty_state_html) { 'No changes' } + + before do + allow(controller_instance).to receive_messages(resource: mock_resource, response: response) + allow(controller_instance).to receive(:view_context) + allow(mock_resource).to receive(:diffs_for_streaming).and_return(mock_diffs) + end + + context 'when no diffs and no offset' do + before do + allow(controller_instance).to receive(:params).and_return(ActionController::Parameters.new) + allow(RapidDiffs::EmptyStateComponent).to receive_message_chain(:new, :render_in).and_return(empty_state_html) + end + + it 'renders empty state' do + controller_instance.send(:stream_diff_files, {}) + + expect(response.stream).to have_received(:write).with(empty_state_html) + end + end + + context 'when offset is provided' do + before do + allow(controller_instance).to receive(:params).and_return(ActionController::Parameters.new(offset: '5')) + end + + it 'does not render empty state' do + expect(RapidDiffs::EmptyStateComponent).not_to receive(:new) + + controller_instance.send(:stream_diff_files, {}) + end + end + end end