Redesign: main layout, header & footer#9340
Merged
ahukkanen merged 91 commits intofeature/redesignfrom Jun 29, 2022
Merged
Conversation
a9c6dcb to
f2b6f21
Compare
70381a6 to
94e04d5
Compare
1380974 to
5b605fa
Compare
0aa4eda to
d93d829
Compare
72263c8 to
4bc03c2
Compare
b62877f to
22a676b
Compare
ff24826 to
8177be8
Compare
bd4450e to
53c52c8
Compare
ebd3316 to
d6f5ced
Compare
53c52c8 to
181ffdf
Compare
Merged
77b5dc4 to
09de3dc
Compare
Merged
Contributor
|
I've been giving this a spin and here is my feedback from the first round of review. Overall this is a bit difficult to review as it also includes changes outside of the header/footer elements which is totally understandable. I've also looked through the code and noticed some potential issues that are not directly related to the header/footer elements and I've included them in a separate section below. Generally it's a good start but I think specifically we need to address the a11y issues as early as possible because otherwise they will be never addressed. I also understand it is still work in progress but I have just been looking at it from a critical point of view and trying to find any issues that I could find. Header and footerGeneral problems
Accessibility issues
Other general feedback (not header/footer)These are more of discussion points, not directly related to header/footer, but I've also included these here as I glanced through the code too:
These were some pointers that we discussed in the initial redesign discussion, so I thought to bring them up already at this stage. |
Contributor
Author
May I know why is incorrect? Decidim a11y validator does not complain about these structures, main browsers have a wide support of this tech, hence you can browse through them using the keyboard and, you don't need any javascript handler since it's native. As the details-summary block is just an structure, you can add a button inside the summary to trigger the hidden content, or you can transform the summary into a button role. Accordingly the MDN docs, semantically is a good approach to our purpose: The Y'day I came accross this article (Jan '19) encouraging the use due to the wide support. Any remaining issues (NVDA-related) there point, to this day they've been solved. IMO, there are more pros than cons using this. |
Contributor
|
@Crashillo We just discussed in the maintainers meeting that we are going to merge this once the CI is green if this is a blocker for other PRs. The failed ones seemed to be flaky so I've restarted them. I'll see if they pass and merge it after that. After that we are going to merge redesign to develop once that's marked completed and CI is green there as well. We can track those issues mentioned above separately and then continue working on them later. |
entantoencuanto
added a commit
that referenced
this pull request
Jun 29, 2022
* [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>
ahukkanen
pushed a commit
that referenced
this pull request
Jul 7, 2022
…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 (#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 (#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>
eliegaboriau
pushed a commit
to eliegaboriau/decidim
that referenced
this pull request
Oct 25, 2022
…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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎩 What? Why?
Refactor Header & Footer to apply new design.
Caveats
It would require add a javascript file to toggle the desktop dropdowns (main & secondary), but this behaviour could be rethinking in a generic way, so I'll postpose it to an individual PR.IncludedScreenshot