- Notifications
You must be signed in to change notification settings - Fork413
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Fix clickable area in dropdowns#12964
Fix clickable area in dropdowns#12964
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is an improvement on what we have, but doesn't actually complies with this part of the original bug report:
I would expect the whole area highlighted with blue color to be clickable.
There's a part in the sides that you can see the blue color, but it isn't clickable:
As far as I know, this needs to be changed in the HTML and can't be done with CSS only (or there's adisplay property that I don't know!). This is what I come up with:
diff --git a/decidim-admin/app/packs/stylesheets/decidim/admin/_item_show.scss b/decidim-admin/app/packs/stylesheets/decidim/admin/_item_show.scssindex 75b4f04017..d55dfd240f 100644--- a/decidim-admin/app/packs/stylesheets/decidim/admin/_item_show.scss+++ b/decidim-admin/app/packs/stylesheets/decidim/admin/_item_show.scss@@ -12,8 +12,8 @@ &-title { @apply text-xl font-bold block sm:flex items-center justify-between gap-4 w-full first-letter:capitalize first:[&>div]:ml-0 first:[&>*]:ml-auto;- .dropdown-pane li a {- @apply font-normal block;+ .dropdown-pane a li {+ @apply font-normal; } } }diff --git a/decidim-admin/app/views/decidim/admin/components/index.html.erb b/decidim-admin/app/views/decidim/admin/components/index.html.erbindex fb57284711..cf6135f5c8 100644--- a/decidim-admin/app/views/decidim/admin/components/index.html.erb+++ b/decidim-admin/app/views/decidim/admin/components/index.html.erb@@ -14,7 +14,11 @@ <div data-dropdown data-auto-focus="true" data-close-on-click="true"> <ul> <% @manifests.each do |manifest| %>- <li><%= link_to t("#{manifest.name}.name", scope: "decidim.components"), new_component_path(type: manifest.name), class: manifest.name %></li>+ <%= link_to new_component_path(type: manifest.name), class: manifest.name do %>+ <li>+ <%= t("#{manifest.name}.name", scope: "decidim.components") %>+ </li>+ <% end %> <% end %> </ul> </div>
You'll need to also apply it to the other dropdowns too:
$ grep -r "menu add-components" decidim-*decidim-admin/app/views/decidim/admin/components/index.html.erb: <ul>decidim-admin/app/views/decidim/admin/exports/_dropdown.html.erb: <ul>decidim-admin/app/views/decidim/admin/shared/landing_page/_content_blocks.html.erb: <ul>decidim-admin/app/views/decidim/admin/imports/_dropdown.html.erb: <ul>decidim-admin/app/views/decidim/admin/imports/new.html.erb: <ul>decidim-conferences/app/views/decidim/conferences/admin/conference_registrations/index.html.erb: <ul>decidim-initiatives/app/views/decidim/initiatives/admin/exports/_dropdown.html.erb: <ul>decidim-meetings/app/views/decidim/meetings/admin/registrations/edit.html.erb: <ul>Thank you@andreslucena I didn't examine the corners or 'sides' in the option which are not clickable, although its encouraging to know we're on a to a start with overall improvement. I can test out your suggested changes in this PR in regards to a fix for this initial bug report, however regarding it affecting other modules, wouldn't it better if this was opened in a new PR to better track the changes made in each module?? Keen to get your thoughts. |
Let's do this in this same PR. Because it will only affect eight dropdowns, you can do them all at the same time. |
There seems to be an issue with the re-factor of Reverting the file in commit:29adce3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I have two improvements to the changes, can you check them out 🙏🏽 ?
decidim-admin/app/packs/stylesheets/decidim/admin/_item_show.scss OutdatedShow resolvedHide resolved
decidim-accountability/app/views/decidim/accountability/admin/results/index.html.erb OutdatedShow resolvedHide resolved
Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>
…results/index.html.erbCo-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>
@andreslucena I'm not receiving any errors locally in Rubocop. Not sure why that suggestion last week is now causing lint errors. Apart from that, its ready to review again. |
The error is from erblint and not from rubocop, so you need to run
|
* dropdown click on components area work* components drop down in porcesses update* dropdown component for initatives complete* exports all component fixed* content blocks components dropdown working* lint syntax code fix* imports new refactored* import dropdown on bulk_actions within proposals refactored* import dropdown on bulk_actions within budgets refactored* import from accountability working within dropdown* conference registerations complete* meetings registrations completed* update to spec tests* removal of expectation link* improvement to link creatiomn in imports* revert changes to exports helper* lint issue* reverting _content_blocks.html file* Update decidim-admin/app/packs/stylesheets/decidim/admin/_item_show.scssCo-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>* Update decidim-accountability/app/views/decidim/accountability/admin/results/index.html.erbCo-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>* erblint changes---------Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>
🎩 What? Why?
Previously clicking on a component would only in the drop down would only work by clicking the title name. Now the whole area is accessible to be clicked.
The
vertical menu add-componentshas also been identified under the following components:📌 Related Issues
Testing
📷 Screenshots
Before:
After:
