From 5c0a10699a8ac0de80f14f07fa58dba5b8438449 Mon Sep 17 00:00:00 2001 From: decabeza Date: Mon, 26 Feb 2018 18:37:30 +0100 Subject: [PATCH 1/7] Refactors documents partials and reorganices css --- app/assets/stylesheets/layout.scss | 79 ++++++++++------------- app/assets/stylesheets/participation.scss | 12 +--- app/views/documents/_document.html.erb | 44 ++++++------- app/views/documents/_documents.html.erb | 16 ++--- config/locales/en/documents.yml | 5 +- config/locales/es/documents.yml | 5 +- 6 files changed, 68 insertions(+), 93 deletions(-) diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss index cebd8c8eb..499a64e49 100644 --- a/app/assets/stylesheets/layout.scss +++ b/app/assets/stylesheets/layout.scss @@ -89,6 +89,10 @@ a { &.warning:hover { color: #000; } + + &.medium { + font-size: $small-font-size; + } } .button.hollow { @@ -2310,61 +2314,48 @@ table { // 20. Documents // ------------- -.documents-list { +.documents { - table { - border: 0; + h2 { + font-size: rem-calc(24); + + span { + color: #4f4f4f; + font-weight: normal; + } } - td { + ul li { + padding-top: $line-height / 2; + + &:not(:first-child) { + border-top: 1px solid $highlight; + } + } + + .document-link { + background: $highlight-soft; + border: 2px solid $highlight; + border-radius: rem-calc(5); + display: block; + margin: $line-height / 2 0; + padding: 0 $line-height / 2; position: relative; - @include breakpoint(small) { - float: left; - width: 100%; - } - - @include breakpoint(medium) { - float: none; - } - a { - width: 100%; + word-wrap: break-word; } - &:first-child { - padding-left: $line-height * 1.5; - - @include breakpoint(small) { - width: 100%; - } - - @include breakpoint(medium) { - width: 70%; - } - - @include breakpoint(large) { - width: 80%; - } - } - - &:first-child::before { + .icon-document { color: #007bb7; - content: 'G'; - font-family: "icons" !important; + display: inline-block; font-size: rem-calc(24); - left: rem-calc(6); - position: absolute; - top: 0; - - @include breakpoint(small) { - padding-top: rem-calc(12); - } - - @include breakpoint(medium) { - padding-top: rem-calc(22); - } + line-height: $line-height; + vertical-align: middle; + } + p { + margin-bottom: 0; } } } diff --git a/app/assets/stylesheets/participation.scss b/app/assets/stylesheets/participation.scss index aa1259bb3..a26b4a5a7 100644 --- a/app/assets/stylesheets/participation.scss +++ b/app/assets/stylesheets/participation.scss @@ -457,7 +457,6 @@ line-height: rem-calc(30); } - .document-link, .video-link { background: $highlight-soft; border: 1px solid $highlight; @@ -470,21 +469,14 @@ word-wrap: break-word; } - [class^="icon-"] { + .icon-video { + color: #cc181e; display: inline-block; font-size: rem-calc(24); line-height: $line-height; vertical-align: middle; } - .icon-document { - color: #007bb7; - } - - .icon-video { - color: #cc181e; - } - p { margin-bottom: 0; } diff --git a/app/views/documents/_document.html.erb b/app/views/documents/_document.html.erb index 47e894aeb..bf549e92b 100644 --- a/app/views/documents/_document.html.erb +++ b/app/views/documents/_document.html.erb @@ -1,24 +1,20 @@ - - - - - - - - -
- <%= document.title %> - - <%= link_to t('documents.buttons.download_document'), - document.attachment.url, - target: "_blank", - rel: "nofollow", - class: 'button hollow' %> - - <% if can?(:destroy, document) %> - <%= link_to t('documents.buttons.destroy_document'), - document_path(document, from: request.url), method: :delete, - data: { confirm: t('documents.actions.destroy.confirm') }, - class: 'button hollow alert' %> - <% end %> -
+
  • + <%= link_to t("documents.buttons.download_document"), + document.attachment.url, target: "_blank", + rel: "nofollow", class: "button hollow medium float-right" %> + + <%= document.title %> +
    + + <%= document.humanized_content_type %> |  + <%= number_to_human_size(document.attachment_file_size, precision: 2) %> + + + <% if can?(:destroy, document) %> +
    + <%= link_to t("documents.buttons.destroy_document"), + document_path(document, from: request.url), method: :delete, + data: { confirm: t("documents.actions.destroy.confirm") }, + class: "delete" %> + <% end %> +
    • diff --git a/app/views/documents/_documents.html.erb b/app/views/documents/_documents.html.erb index 4387817d6..5963127df 100644 --- a/app/views/documents/_documents.html.erb +++ b/app/views/documents/_documents.html.erb @@ -1,11 +1,9 @@ -
    -
    - <% if documents.any? %> +
    +

    <%= t("documents.title") %> (<%= documents.count %>)

    + + <% if documents.any? %> +
      <%= render partial: "documents/document", collection: documents %> - <% else %> -
      - <%= t('documents.no_documents') %> -
      - <% end %> -
    + + <% end %>
    diff --git a/config/locales/en/documents.yml b/config/locales/en/documents.yml index 7db70b25a..4cd868d40 100644 --- a/config/locales/en/documents.yml +++ b/config/locales/en/documents.yml @@ -1,7 +1,6 @@ en: documents: - tab: Documents - no_documents: Don't have uploaded documents + title: Documents max_documents_allowed_reached_html: You have reached the maximum number of documents allowed! You have to delete one before you can upload another. form: title: Documents @@ -17,7 +16,7 @@ en: confirm: Are you sure you want to delete the document? This action cannot be undone! buttons: download_document: Download file - destroy_document: Destroy + destroy_document: Destroy document errors: messages: in_between: must be in between %{min} and %{max} diff --git a/config/locales/es/documents.yml b/config/locales/es/documents.yml index 0e57897e6..3e2c0e3bd 100644 --- a/config/locales/es/documents.yml +++ b/config/locales/es/documents.yml @@ -1,7 +1,6 @@ es: documents: - tab: Documentos - no_documents: No hay documentos subidos + title: Documentos max_documents_allowed_reached_html: '¡Has alcanzado el número máximo de documentos permitidos! Tienes que eliminar uno antes de poder subir otro.' form: title: Documentos @@ -17,7 +16,7 @@ es: confirm: '¿Está seguro de que desea eliminar el documento? Esta acción no se puede deshacer!' buttons: download_document: Descargar archivo - destroy_document: Eliminar + destroy_document: Eliminar documento errors: messages: in_between: debe estar entre %{min} y %{max} From 466f8ef4e08683bfa3421b10c56a285af1ceaef5 Mon Sep 17 00:00:00 2001 From: decabeza Date: Mon, 26 Feb 2018 18:37:43 +0100 Subject: [PATCH 2/7] Removes unnecessary css From 5b7a5a2b4b93ba37c340548b3a0826d32a2dd803 Mon Sep 17 00:00:00 2001 From: decabeza Date: Mon, 26 Feb 2018 18:38:05 +0100 Subject: [PATCH 3/7] Removes document tab on proposals --- app/views/proposals/_filter_subnav.html.erb | 8 -------- app/views/proposals/show.html.erb | 10 ++++------ 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/app/views/proposals/_filter_subnav.html.erb b/app/views/proposals/_filter_subnav.html.erb index fece62a17..6347d56b0 100644 --- a/app/views/proposals/_filter_subnav.html.erb +++ b/app/views/proposals/_filter_subnav.html.erb @@ -17,14 +17,6 @@ <% end %> -
  • - <%= link_to "#tab-documents" do %> -

    - <%= t("documents.tab") %> - (<%= @proposal.documents.count %>) -

    - <% end %> -
    • diff --git a/app/views/proposals/show.html.erb b/app/views/proposals/show.html.erb index dbeb110ce..e2a7d0c17 100644 --- a/app/views/proposals/show.html.erb +++ b/app/views/proposals/show.html.erb @@ -106,6 +106,10 @@ <% end %> + <%= render 'documents/documents', + documents: @proposal.documents, + max_documents_allowed: Proposal.max_documents_allowed %> + <%= render 'shared/tags', taggable: @proposal %> <%= render 'shared/geozone', geozonable: @proposal %> @@ -206,10 +210,4 @@
    <%= render "proposals/comments" %>
    - -
    - <%= render 'documents/documents', - documents: @proposal.documents, - max_documents_allowed: Proposal.max_documents_allowed %> -
    From 46f21330bda362ae0e2d53a77501a2a05f8b4b0d Mon Sep 17 00:00:00 2001 From: decabeza Date: Mon, 26 Feb 2018 18:38:23 +0100 Subject: [PATCH 4/7] Removes document tab on budgets investments --- app/views/budgets/investments/_filter_subnav.html.erb | 8 -------- app/views/budgets/investments/_investment_show.html.erb | 4 ++++ app/views/budgets/investments/show.html.erb | 7 ------- 3 files changed, 4 insertions(+), 15 deletions(-) diff --git a/app/views/budgets/investments/_filter_subnav.html.erb b/app/views/budgets/investments/_filter_subnav.html.erb index 01fef4657..a46c33b2d 100644 --- a/app/views/budgets/investments/_filter_subnav.html.erb +++ b/app/views/budgets/investments/_filter_subnav.html.erb @@ -17,14 +17,6 @@ <% end %> -
  • - <%= link_to "#tab-documents" do %> -

    - <%= t("documents.tab") %> - (<%= @investment.documents.count %>) -

    - <% end %> -
    • diff --git a/app/views/budgets/investments/_investment_show.html.erb b/app/views/budgets/investments/_investment_show.html.erb index addc67adc..782c26a4f 100644 --- a/app/views/budgets/investments/_investment_show.html.erb +++ b/app/views/budgets/investments/_investment_show.html.erb @@ -51,6 +51,10 @@

    <% end %> + <%= render 'documents/documents', + documents: investment.documents, + max_documents_allowed: Budget::Investment.max_documents_allowed %> + <%= render 'shared/tags', taggable: investment %> <% if investment.external_url.present? %> diff --git a/app/views/budgets/investments/show.html.erb b/app/views/budgets/investments/show.html.erb index e55e88510..264e7b102 100644 --- a/app/views/budgets/investments/show.html.erb +++ b/app/views/budgets/investments/show.html.erb @@ -19,11 +19,4 @@ comment_flags: @comment_flags, display_comments_count: false } %> - -
    - <%= render 'documents/documents', - documents: @investment.documents, - max_documents_allowed: Budget::Investment.max_documents_allowed %> -
    - From 1546f67367c5e6ec6b49fd3806f8152d0cf145f7 Mon Sep 17 00:00:00 2001 From: decabeza Date: Wed, 28 Feb 2018 16:21:08 +0100 Subject: [PATCH 5/7] Removes document tab on legislation proposals --- .../legislation/proposals/_filter_subnav.html.erb | 8 -------- app/views/legislation/proposals/show.html.erb | 10 ++++------ 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/app/views/legislation/proposals/_filter_subnav.html.erb b/app/views/legislation/proposals/_filter_subnav.html.erb index 986af6463..0250d36db 100644 --- a/app/views/legislation/proposals/_filter_subnav.html.erb +++ b/app/views/legislation/proposals/_filter_subnav.html.erb @@ -9,14 +9,6 @@ <% end %> -
  • - <%= link_to "#tab-documents" do %> -

    - <%= t("documents.tab") %> - (<%= @proposal.documents.count %>) -

    - <% end %> -
    • diff --git a/app/views/legislation/proposals/show.html.erb b/app/views/legislation/proposals/show.html.erb index 8ad9e4f54..0e71e6f7f 100644 --- a/app/views/legislation/proposals/show.html.erb +++ b/app/views/legislation/proposals/show.html.erb @@ -80,6 +80,10 @@

    <%= @proposal.question %>

    + <%= render 'documents/documents', + documents: @proposal.documents, + max_documents_allowed: Proposal.max_documents_allowed %> + <%= render 'shared/tags', taggable: @proposal %> <%= render 'shared/geozone', geozonable: @proposal %> @@ -112,10 +116,4 @@
    <%= render "legislation/proposals/comments" %>
    - -
    - <%= render 'documents/documents', - documents: @proposal.documents, - max_documents_allowed: Proposal.max_documents_allowed %> -
    From e79f64c00998e55257ad7aa9dcfc2009237177ea Mon Sep 17 00:00:00 2001 From: decabeza Date: Tue, 27 Feb 2018 12:56:15 +0100 Subject: [PATCH 6/7] Updates specs --- spec/shared/features/documentable.rb | 53 +++++++-------------- spec/shared/features/nested_documentable.rb | 2 - 2 files changed, 18 insertions(+), 37 deletions(-) diff --git a/spec/shared/features/documentable.rb b/spec/shared/features/documentable.rb index f480bc2cb..3b3520267 100644 --- a/spec/shared/features/documentable.rb +++ b/spec/shared/features/documentable.rb @@ -15,30 +15,24 @@ shared_examples "documentable" do |documentable_factory_name, documentable_path, end end - context "Show documents tab" do + context "Show documents" do scenario "Download action should be able to anyone" do visit send(documentable_path, arguments) - within "#tab-documents" do - expect(page).to have_link("Download file") - end + expect(page).to have_link("Download file") end scenario "Download file link should have blank target attribute" do visit send(documentable_path, arguments) - within "#tab-documents" do - expect(page).to have_selector("a[target=_blank]", text: "Download file") - end + expect(page).to have_selector("a[target=_blank]", text: "Download file") end scenario "Download file links should have rel attribute setted to no follow" do visit send(documentable_path, arguments) - within "#tab-documents" do - expect(page).to have_selector("a[rel=nofollow]", text: "Download file") - end + expect(page).to have_selector("a[rel=nofollow]", text: "Download file") end describe "Destroy action" do @@ -46,36 +40,28 @@ shared_examples "documentable" do |documentable_factory_name, documentable_path, scenario "Should not be able when no user logged in" do visit send(documentable_path, arguments) - within "#tab-documents" do - expect(page).not_to have_link("Destroy") - end + expect(page).not_to have_link("Destroy document") end scenario "Should be able when documentable author is logged in" do login_as documentable.author visit send(documentable_path, arguments) - within "#tab-documents" do - expect(page).to have_link("Destroy") - end + expect(page).to have_link("Destroy document") end scenario "Administrators cannot destroy documentables they have not authored" do login_as(administrator) visit send(documentable_path, arguments) - within "#tab-documents" do - expect(page).not_to have_link("Destroy") - end + expect(page).not_to have_link("Destroy document") end scenario "Users cannot destroy documentables they have not authored" do login_as(create(:user)) visit send(documentable_path, arguments) - within "#tab-documents" do - expect(page).not_to have_link("Destroy") - end + expect(page).not_to have_link("Destroy document") end end @@ -88,10 +74,9 @@ shared_examples "documentable" do |documentable_factory_name, documentable_path, login_as documentable.author visit send(documentable_path, arguments) - within "#tab-documents" do - within "#document_#{document.id}" do - click_on "Destroy" - end + + within "#document_#{document.id}" do + click_on "Destroy document" end expect(page).to have_content "Document was deleted successfully." @@ -101,23 +86,21 @@ shared_examples "documentable" do |documentable_factory_name, documentable_path, login_as documentable.author visit send(documentable_path, arguments) - within "#tab-documents" do - within "#document_#{document.id}" do - click_on "Destroy" - end + + within "#document_#{document.id}" do + click_on "Destroy document" end - expect(page).to have_link "Documents (0)" + expect(page).to have_content "Documents (0)" end scenario "Should redirect to documentable path after successful deletion" do login_as documentable.author visit send(documentable_path, arguments) - within "#tab-documents" do - within "#document_#{document.id}" do - click_on "Destroy" - end + + within "#document_#{document.id}" do + click_on "Destroy document" end within "##{dom_id(documentable)}" do diff --git a/spec/shared/features/nested_documentable.rb b/spec/shared/features/nested_documentable.rb index 02dd1e54d..2e5db8228 100644 --- a/spec/shared/features/nested_documentable.rb +++ b/spec/shared/features/nested_documentable.rb @@ -195,8 +195,6 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na documentable_redirected_to_resource_show_or_navigate_to expect(page).to have_content "Documents" - - find("#tab-documents-label").click expect(page).to have_content "empty.pdf" # Review From 53e36850a4e06361e8ae9e5bda6fbd005b9b26cd Mon Sep 17 00:00:00 2001 From: decabeza Date: Tue, 27 Feb 2018 13:37:32 +0100 Subject: [PATCH 7/7] Fixes documentable rubocop offences --- spec/shared/features/documentable.rb | 14 +++++---- spec/shared/features/nested_documentable.rb | 33 ++++++++++++++------- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/spec/shared/features/documentable.rb b/spec/shared/features/documentable.rb index 3b3520267..16fab74ce 100644 --- a/spec/shared/features/documentable.rb +++ b/spec/shared/features/documentable.rb @@ -1,11 +1,13 @@ -shared_examples "documentable" do |documentable_factory_name, documentable_path, documentable_path_arguments| +shared_examples "documentable" do |documentable_factory_name, + documentable_path, + documentable_path_arguments| include ActionView::Helpers - let(:administrator) { create(:user) } - let(:user) { create(:user) } - let(:arguments) { {} } - let(:documentable) { create(documentable_factory_name, author: user) } - let!(:document) { create(:document, documentable: documentable, user: documentable.author) } + let(:administrator) { create(:user) } + let(:user) { create(:user) } + let(:arguments) { {} } + let(:documentable) { create(documentable_factory_name, author: user) } + let!(:document) { create(:document, documentable: documentable, user: documentable.author) } before do create(:administrator, user: administrator) diff --git a/spec/shared/features/nested_documentable.rb b/spec/shared/features/nested_documentable.rb index 2e5db8228..c8264420c 100644 --- a/spec/shared/features/nested_documentable.rb +++ b/spec/shared/features/nested_documentable.rb @@ -1,4 +1,7 @@ -shared_examples "nested documentable" do |login_as_name, documentable_factory_name, path, documentable_path_arguments, fill_resource_method_name, submit_button, documentable_success_notice| +shared_examples "nested documentable" do |login_as_name, documentable_factory_name, + path, documentable_path_arguments, + fill_resource_method_name, submit_button, + documentable_success_notice| include ActionView::Helpers include DocumentsHelper include DocumentablesHelper @@ -26,7 +29,8 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na expect(page).to have_css "#new_document_link", visible: true end - scenario "Should not show new document link when documentable max documents allowed limit is reached", :js do + scenario "Should not show new document link when + documentable max documents allowed limit is reached", :js do login_as user_to_login visit send(path, arguments) @@ -81,7 +85,8 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na expect(page).to have_css ".file-name", text: "empty.pdf" end - scenario "Should update nested document file title with file name after choosing a file when no title defined", :js do + scenario "Should update nested document file title with + file name after choosing a file when no title defined", :js do login_as user_to_login visit send(path, arguments) @@ -90,7 +95,8 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na expect_document_has_title(0, "empty.pdf") end - scenario "Should not update nested document file title with file name after choosing a file when title already defined", :js do + scenario "Should not update nested document file title with + file name after choosing a file when title already defined", :js do login_as user_to_login visit send(path, arguments) @@ -141,7 +147,8 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na expect_document_has_cached_attachment(0, "") end - scenario "Should show document errors after documentable submit with empty document fields", :js do + scenario "Should show document errors after documentable submit with + empty document fields", :js do login_as user_to_login visit send(path, arguments) @@ -163,7 +170,8 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na expect(page).not_to have_css("#nested-documents .document") end - scenario "Should show successful notice when resource filled correctly without any nested documents", :js do + scenario "Should show successful notice when + resource filled correctly without any nested documents", :js do login_as user_to_login visit send(path, arguments) @@ -173,7 +181,8 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na expect(page).to have_content documentable_success_notice end - scenario "Should show successful notice when resource filled correctly and after valid file uploads", :js do + scenario "Should show successful notice when + resource filled correctly and after valid file uploads", :js do login_as user_to_login visit send(path, arguments) send(fill_resource_method_name) if fill_resource_method_name @@ -202,13 +211,16 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na expect(page).to have_css("a[href$='.pdf']") end - scenario "Should show resource with new document after successful creation with maximum allowed uploaded files", :js do + scenario "Should show resource with new document after successful creation with + maximum allowed uploaded files", :js do login_as user_to_login visit send(path, arguments) send(fill_resource_method_name) if fill_resource_method_name - documentable.class.max_documents_allowed.times { documentable_attach_new_file(cycle(Dir.glob('spec/fixtures/files/*.pdf'))) } + documentable.class.max_documents_allowed.times { + documentable_attach_new_file(cycle(Dir.glob('spec/fixtures/files/*.pdf'))) + } click_on submit_button documentable_redirected_to_resource_show_or_navigate_to @@ -226,7 +238,8 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na expect(page).to have_css ".document", count: 1 end - scenario "Should not show add document button when documentable has reached maximum of documents allowed", :js do + scenario "Should not show add document button when + documentable has reached maximum of documents allowed", :js do login_as user_to_login create_list(:document, documentable.class.max_documents_allowed, documentable: documentable) visit send(path, arguments)