Redesign / Introduce concern to pick redesign layouts when activate by ferblape · Pull Request #9229 · decidim/decidim

ferblape · 2022-05-04T08:40:19Z

This PR is the base branch where the redesign is integrated. It could be understood as the develop branch of the redesign process.

Read more about the redesign in this page for contributors

The changes happening in this branch are described below:

How to enable redesign

By default the use of redesign files is disabled, even if they are expressly activated from a controller. To allow the use of these files there must be an environment variable named DECIDIM_REDESIGN_ACTIVE set to true or this value can be set directly in the application using Decidim in config/initializers/decidim.rb replacing the existing line:

Decidim.configure do |config|
# ...
  config.redesign_active = Rails.application.secrets.decidim[:redesign_active] if Rails.application.secrets.decidim[:redesign_active].present?
# ...

with:

Decidim.configure do |config|
# ...
  config.redesign_active = true
# ...

New redesign layout

A new layout with a couple of new partials has been created at decidim-core/app/views/layouts/decidim/redesigned_application.html.erb (for the moment just two partials because it's a proof of concept).

The convention we're following is that the new files will be prefixed with redesigned_, so at the end of the redesign it'll be easy to identify these files and remove the preffx from them.

This new layout references to the new JS and CSS packs.

Redesign javascript packs

This new pack decidim-core/app/packs/entrypoints/redesigned_decidim_core.js is just a copy of the existing decidim_core.js and references the same files. We'll be creating the redesigned_ version of the js files only when it's necessary, while the idea is to continue working with the existing files.

Redesign CSS packs

Same as for javascript, a new pack has been created at decidim-core/app/packs/stylesheets/decidim/redesigned_application.scss

While in JS we expect few changes, in CSS almost every file is going to change, so we expect to have a lot of redesigned_*.css files.

Redesign concern

A new concern has been created to activate the new layout in a controller, just by adding it to the application controller file of the engine (there are some things to have into consideration for some engines such as participatory spaces, which have an special layout but we'll tackle later).

You can see an example at decidim-core/app/controllers/decidim/homepage_controller.rb

Rename task

A WIP rake task is being developed to automate the final file renaming. The idea behind this task is prevent any human error in the process of moving all the redesign templates, styles and javascripts to the original names.

decidim-admin/app/controllers/decidim/admin/application_controller.rb

decidim-assemblies/app/controllers/decidim/assemblies/assemblies_controller.rb

decidim-assemblies/app/views/layouts/decidim/redesigned_assembly.html.erb

decidim-core/app/controllers/concerns/decidim/devise_controllers.rb

decidim-core/app/controllers/decidim/application_controller.rb

decidim-core/app/views/layouts/decidim/redesigned_application.html.erb

decidim-meetings/app/packs/stylesheets/decidim/meetings/_redesigned_meetings.scss

ferblape · 2022-05-20T05:53:17Z

@decidim/maintainers this is ready to review. We'd like this to be merged in develop to simplify further development of the redesign features

ahukkanen · 2022-06-29T13:20:32Z

@ferblape @Crashillo @furilo I just merged #9340 into this branch.

Can you please fix the conflicts, mark this ready and I'll merge this into develop?

We agreed today that we are going to merge these blocking PRs first and then handle the issues from the review separately.

entantoencuanto · 2022-07-01T13:56:53Z

Hi, @ahukkanen, now is ready for review after last updates

ahukkanen

I have checked this through and left some new comments along with the comments I provided at #9340 which lists the major issues I noticed during testing it in action.

I've had another go and also left few comments regarding some lines of code.

As agreed, I will merge this for now and leave the comments open for your review for the upcoming PRs.

ahukkanen · 2022-07-07T13:46:02Z

decidim-core/app/helpers/decidim/paginate_helper.rb

-      paginate collection, theme: "decidim", params: paginate_params
+      return if collection.total_pages == 1
+
+      content_tag :div, class: "flex flex-col-reverse md:flex-row items-center justify-between gap-1 py-8 md:py-16", data: { pagination: "" } do


I would rather see this in a cell/partial in order to make customization easier.

ahukkanen · 2022-07-07T13:48:38Z

decidim-core/app/packs/src/decidim/dropdowns_handler.js

@@ -0,0 +1,13 @@
+/**
+ * Handler to allow ONLY ONE dropdown (details HTML tag) open at once.


As I mentioned in my review of #9340, it is not intended behavior to only have one dropdown open at a time. It may produce confusing experience in some occasions (see the commenta at #9340).

ahukkanen · 2022-07-07T13:58:58Z

decidim-core/app/packs/src/decidim/redesigned_external_link.js

@@ -0,0 +1,32 @@
+import icon from "src/decidim/icon"
+
+export default class ExternalLink {


With this I believe we are losing one useful feature of the current external link elements, i.e. configuring the target element within the link for the external link. This is highly useful when e.g. the whole card is a link (as it generally should be and as it is in some customized instances), to be able to put the external link indicator anywhere within the card.

ahukkanen · 2022-07-07T13:59:36Z

decidim-core/app/packs/stylesheets/decidim/_redesigned_buttons.scss

+    @apply bg-tertiary text-white;
+  }
+
+  &.white{


See comments from #9340.

ahukkanen · 2022-07-07T14:08:42Z

decidim-core/app/views/kaminari/decidim/_page.html.erb

    remote:        data-remote -%>
-<li class="page<%= " current" if page.current? %>"<%== ' aria-current="page"' if page.current? %>>
-  <%= link_to_unless page.current?, page, url, { remote: remote, rel: page.rel, title: t("views.pagination.title") << page.to_s } %>
+<li class="w-6 h-6 rounded-full hover:bg-background-3 transition<%= "mx-2 bg-background-3 grid place-items-center" if page.current? %>" <%== 'aria-current="page"' if page.current? %> data-page>


See comments from #9340.

ahukkanen · 2022-07-07T14:45:18Z

decidim-core/spec/helpers/decidim/endorsable_helper_spec.rb

        allow(controller).to receive(:current_user).and_return(user)
+        allow(controller).to receive(:redesign_enabled?).and_return(redesign_enabled)
+        # rubocop:disable RSpec/AnyInstance
+        allow_any_instance_of(ActionView::Base).to receive(:redesign_enabled?).and_return(redesign_enabled)


Could we move this to a shared context that is included in the target spec as this seems to be repeated across multiple specs?

ahukkanen · 2022-07-07T14:45:23Z

decidim-core/spec/helpers/decidim/icon_helper_spec.rb


+      before do
+        # rubocop:disable RSpec/AnyInstance
+        allow_any_instance_of(ActionView::Base).to receive(:redesign_enabled?).and_return(redesign_enabled)


Could we move this to a shared context that is included in the target spec as this seems to be repeated across multiple specs?

ahukkanen · 2022-07-07T14:45:30Z

decidim-core/spec/helpers/decidim/widget_urls_helper_spec.rb

+
+    before do
+      # rubocop:disable RSpec/AnyInstance
+      allow_any_instance_of(ActionView::Base).to receive(:redesign_enabled?).and_return(redesign_enabled)


Could we move this to a shared context that is included in the target spec as this seems to be repeated across multiple specs?

ahukkanen · 2022-07-07T14:45:38Z

decidim-proposals/spec/helpers/map_helper_spec.rb

+
+      before do
+        # rubocop:disable RSpec/AnyInstance
+        allow_any_instance_of(ActionView::Base).to receive(:redesign_enabled?).and_return(redesign_enabled)


Could we move this to a shared context that is included in the target spec as this seems to be repeated across multiple specs?

ahukkanen · 2022-07-07T14:46:53Z

decidim-assemblies/app/controllers/decidim/assemblies/assemblies_controller.rb

    # A controller that holds the logic to show Assemblies in a public layout.
    class AssembliesController < Decidim::Assemblies::ApplicationController
      include ParticipatorySpaceContext
+


I would avoid changing files unless it is necessary. Here you probably did some changes at first and then reverted the changes which caused this issue but if you revert stuff, revert back to the original in the future please. 🙏

…ecidim#9229) * Add RedesignLayout concern * Include redesign layout in some controllers * Improve RedesignLayout concern * Add new entrypoint for redesigned decidim_core and use it in dedesigned layout * Enable redesign in homepage controller * Add task to create redesigned entrypoints * Test redesigned scss partial * Test imports of redesigned scss from js entrypoints * Create a redesigned_app group and a specific method to import scss partials * Add task to create redesigned entrypoints * Create a redesigned_app group and a specific method to import scss partials * Improve redesign_entrypoint task to register also scss partials * Redesigned layotu calls redesign CSS packs * Allow cells to use redesigned versions if redesign is enabled in controller and redesigned cell exists * Test redesigned cells in meetings author * Add some redesign helper methods * Delegate some redesign methods on controller on cells * Add some redesign helper methods * include redesign concern in admin controllers with feature disabled * Add redesign layout to devise controllers concern * Disable redesign in all controllers * Disable redesign explicitly in assemblies controller to avoid redesign_participatory_space_layout default behavior * Fix tests * Define a redesign_active Decidim setting and take its value from an environment variable * Remove duplicated line * Use redesign_active initializer setting in redesign concern and use a class variable With a class variable the setting can be inherited by controllers and ovewritten if required with a redesign active: true/false * Remove no longer required redesign_defined? method * Revert redesign in assemblies participatory space * Don't disable redesign in main application controller * Remove testing text * Revert redesign in meetings stylesheets assets * Disable rubocop Style/ClassVars in redesign layout * Move javascript to footer * Load redesigned app styles in redesigned stylesheet * Redesign: Install tailwind (decidim#9274) * [skip ci] testing commit * testing files * Copy tailwind configuration * clean files to run tailwind * remove the package-locks references * restore default import to override * Fix decidim gem path in tailwind configuration * testing files * Remove not required file * Update package-lock * Fix quotes * Update package-lock design app * Update plugin order * Simplify plugins * Fix import order * Define paths that don't include packs/ folder to prevent infinite loops * Revert "Simplify plugins" This reverts commit 1d53c00. * Load node_modules styles with JS * JS offense * CSS offense * Detailed configuration * Use explicit list of paths Avoid using the star in the paths to prevent infinite loops * Add clarification * Remove package-lock file Co-authored-by: Eduardo Martinez Echevarria <eduardomech@gmail.com> Co-authored-by: Fernando Blat <fernando@blat.es> * Revert definition of @@enable_redesign class variable and use an instance variable * Enable redesign by default * Fix redesigned cell name detection * Redesign: main layout, header & footer (decidim#9340) * [skip ci] testing commit * testing files * clean files to run tailwind * remove the package-locks references * Fix decidim gem path in tailwind configuration * testing files * Remove not required file * Fix import order * Load node_modules styles with JS * Use explicit list of paths Avoid using the star in the paths to prevent infinite loops * clean files to run tailwind * typography * add colors & update typography * add new icons support * header desktop-mobile & css toggler * easier legacy css toggler * footer response new desgins * a11y issues * refactor external link js * tailwind config in ERB & fix external_link position * add fonts in new design * buttons component * [skip ci] button hover * first dropdown * position dropdown * sort files into more descriptive folders * second dropdown * [skip ci] second dropdown tablet/mobile * [skip ci] rearrage files * fix header links variants (desktop/mobile) * [skip ci] overlaps among dropdowns * [skip ci] tailwind dropdown component * [skip ci] restore files * most accurate naming * Import ExternalLink from redesigned version * Update selector in tests * Fix external_link test * Remove package-lock from packages * Remove duplicated tailwind config * Enable redesign for main layout * Fix some tests related with icons * Fix some elections tests related with icons * Recover yield in redesigned wrapper * Recover detection of redesign in icon method * Define redesign_enabled? on menu_helper test * Revert "Fix some elections tests related with icons" This reverts commit 09de3dc. * Fix tests to take into account redesign_enabled? * Fix tests to take into account redesign_enabled? * fix stylelint * fix erblint * Disable completely redesign in admin section * fix footer glitches * enhacement decidim paginate * pagination * results per page & some generalizations turn into specifications * fix pagination * two-columns layout * one column layout + decorator * button should be an inline element * update the real tailwind config file * replace status with colors (to match tailwind classes) * label component * fix gray colors tailwind * fix footer color (unique) * Fix translation * fix stylelint * Hide results_per_page if there is only one page * Hide pagination if there is only one page * Add some identifiers to pagination elements * Fix pagination shared examples * new tailwind color setup * update classes accordingly new setup * replace old css classes * Remove unused translations * Increase default per page in pagination on some tests with filters * Fix some pagination tests * Fix default per_page test * Include LayoutHelper in some cells using icon * Fix pagination tests * Fix cell test * Fix pagination test * fix erblint * fix stylelint * fix a11y * fix bug on existing avatar * Add admin dashboard link to main links dropdown * Add URLs to main links in header * Fix path * Fix dropdown * add layout * responsive layout Co-authored-by: Eduardo Martinez Echevarria <eduardomech@gmail.com> Co-authored-by: Fernando Blat <fernando@blat.es> * Replace TODOs * fix center layout fallback only children * Fix test providing redesign_enabled? method * alter tailwind setup * typo * common forms styling * fix lint offenses * Fix links in redesigned_main_links_dropdown Co-authored-by: Eduardo Martinez Echevarria <eduardomech@gmail.com> Co-authored-by: Hugoren Martinako <aumpfbahn@gmail.com>

entantoencuanto force-pushed the feature/redesign branch from 61b6e7a to 7979d75 Compare May 4, 2022 19:07

Crashillo mentioned this pull request May 9, 2022

Redesign: Install tailwind #9274

Merged

entantoencuanto force-pushed the feature/redesign branch from ee73106 to 6f44b36 Compare May 12, 2022 15:12

ahukkanen requested a review from lahdeero May 16, 2022 08:02

entantoencuanto force-pushed the feature/redesign branch 4 times, most recently from 1f91262 to b0c45d3 Compare May 17, 2022 17:17

ferblape commented May 18, 2022

View reviewed changes

furilo added the contract: lot: px label May 18, 2022

entantoencuanto force-pushed the feature/redesign branch from 92a1067 to d588534 Compare May 19, 2022 09:07

ferblape changed the title ~~Redesign WIP (first commits)~~ Redesign / Introduce concern to pick redesign layouts when activate May 19, 2022

ferblape marked this pull request as ready for review May 19, 2022 10:16

ferblape force-pushed the feature/redesign branch 2 times, most recently from 94e04d5 to 2b0e23a Compare May 27, 2022 07:30

ahukkanen removed the request for review from lahdeero May 30, 2022 15:20

ferblape changed the title ~~Redesign / Introduce concern to pick redesign layouts when activate~~ 🚫 Redesign / Introduce concern to pick redesign layouts when activate (DON'T MERGE WITH DEVELOP) May 31, 2022

andreslucena changed the title ~~🚫 Redesign / Introduce concern to pick redesign layouts when activate (DON'T MERGE WITH DEVELOP)~~ 🚫 Redesign / Introduce concern to pick redesign layouts when activate Jun 1, 2022

andreslucena marked this pull request as draft June 1, 2022 09:34

ferblape force-pushed the feature/redesign branch 2 times, most recently from b62877f to 22a676b Compare June 13, 2022 12:53

entantoencuanto force-pushed the feature/redesign branch 4 times, most recently from aaea092 to a8c43ad Compare June 20, 2022 13:41

andreslucena added project: redesign Barcelona City Council contract and removed contract: lot: px labels Jun 21, 2022

Add RedesignLayout concern

f5a2aaa

Crashillo added 2 commits June 30, 2022 20:18

common forms styling

02a5c05

fix lint offenses

954ef24

entantoencuanto marked this pull request as ready for review July 1, 2022 13:55

ferblape changed the title ~~🚫 Redesign / Introduce concern to pick redesign layouts when activate~~ Redesign / Introduce concern to pick redesign layouts when activate Jul 4, 2022

Fix links in redesigned_main_links_dropdown

11bce93

ahukkanen reviewed Jul 7, 2022

View reviewed changes

ahukkanen approved these changes Jul 7, 2022

View reviewed changes

ahukkanen merged commit 5288afe into develop Jul 7, 2022

ahukkanen deleted the feature/redesign branch July 7, 2022 14:51

This was referenced Jul 25, 2022

Broken pagination CSS #9571

Closed

Open in new tab icon is displayed next to logos and social media icons #9612

Closed

alecslupu mentioned this pull request Dec 1, 2023

Remove postcss-loader config loader customization #12103

Merged

		@@ -0,0 +1,13 @@
		/**
		* Handler to allow ONLY ONE dropdown (details HTML tag) open at once.

		@@ -0,0 +1,32 @@
		import icon from "src/decidim/icon"

		export default class ExternalLink {

Uh oh!

Conversation

ferblape commented May 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to enable redesign

New redesign layout

Redesign javascript packs

Redesign CSS packs

Redesign concern

Rename task

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ferblape commented May 20, 2022

Uh oh!

ahukkanen commented Jun 29, 2022

Uh oh!

entantoencuanto commented Jul 1, 2022

Uh oh!

ahukkanen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ferblape commented May 4, 2022 •

edited

Loading