Artificial margin between proposal "header" and list of endorsements

[deivid-rodriguez]

This is a Bug Report

🎩 Description

I see an excesive white space on top of the list of endorsements for a proposal. It seems like that whitespace is controlled by the height of the sidebar, which seems unexpected, since adding things to the sidebar will keep unintentionally increasing the whitespace...

Setting height: 0 on the sidebar seems to do the trick on my browser, not sure what consequences that could have. However, the list of endorsements seemed "out of the markup structure" to me, the following patch in the template also had a similar visual effect of removing the whitespace. Not sure if that could have other negative consequences or break anything:

diff --git a/decidim-proposals/app/views/decidim/proposals/proposals/show.html.erb b/decidim-proposals/app/views/decidim/proposals/proposals/show.html.erb
index 5a0e141fc..7216f4522 100644
--- a/decidim-proposals/app/views/decidim/proposals/proposals/show.html.erb
+++ b/decidim-proposals/app/views/decidim/proposals/proposals/show.html.erb
@@ -90,12 +90,16 @@
     <%= linked_resources_for @proposal, :projects, "included_proposals" %>
     <%= linked_resources_for @proposal, :meetings, "proposals_from_meeting" %>
     <%= linked_resources_for @proposal, :proposals, "copied_from_component" %>
+
+    <div class="section">
+      <a name="list-of-endorsements"></a>
+      <%= render partial: 'endorsements_listing', locals: {proposal: @proposal} %>
+    </div>
   </div>
 </div>
+
 <%= attachments_for @proposal %>
 
-<a name="list-of-endorsements"></a>
-<%= render partial: 'endorsements_listing', locals: {proposal: @proposal} %>
 <%= comments_for @proposal %>
 
 <%= javascript_include_tag "decidim/proposals/social_share" %>

📌 Related issues

• Not sure.

📋 Additional Data

I can only reproduce this issue on chromium, but doesn't happen on firefox or chrome. Not sure about other browsers.

[mrcasals]

@deivid-rodriguez maybe we could move the attachments too?

[deivid-rodriguez]

Yeah, probably, otherwise the list of attachments will probably get the same whitespace on top of it...

[oriolgual]

Pinging @decidim/web-design

[htmlboy]

This structure was built for longer content cases, and making sure the "voting block" would stay relatively on top on mobile devices. You could easily make the "List of endorsements" block part of the main column (thus avoiding the white gap), but you should probably reorganise the voting block for mobile devices with JS.

[deivid-rodriguez]

I think I understand what you're saying @htmlboy. Moving attachments and endorsment list to the upper block means the voting sidebar will be rendered after that on mobile, and that's not good/intented. 👍

The current HTML structure still could be improved but maybe the quick fix for this is to set height: 0 on the sidebar like I did in the gif? Do you think that could cause any problems?

[htmlboy]

@deivid-rodriguez Height:0 sounds quite risky, since it won't probably force a scroll behaviour (or push other content) when the right column is longer than the main column. I'd test in several cases before using it…

[oriolgual]

Maybe I should have mentioned @Crashillo and @javierarce 😅

[deivid-rodriguez]

I'm sure someone knows a good solution for this that works consistently across browsers. ☺

[Crashillo]

I'm taking a look at the markup in master, and it seems that the list-of-endorsements is misplaced. It should be located in the same container the proposal text is.
I don't understand the misbehaviour of the other browsers, but, try what I said, and let's see whether it fixes.

[deivid-rodriguez]

Seems like a better idea indeed.

[xabier]

hi @Crashillo how is this going?

[Crashillo]

I'm missing something here. Regarding as the piece of code you @deivid-rodriguez attached in the description, the commit moves the anchor and the partial inside the upper block. That's correct.

Nevertheless, the code in master has not included that change. Has been actually merged? cc/ @decidim/lot-core

This block belongs to a partial/widget/whateveryouwannacallit to render the adhesions list. In the design, that stuff could be found in the initiative-view. If you take a look at the snippet @deivid-rodriguez sent, the commit contains the new changes, so @decidim/lot-mods has already integrate it, hasn't it?
By the way, is this <a name="list-of-endorsements"></a> controlled? An empty anchor?

In any case, I remove the team:design tag as it's totally unrelated.

[deivid-rodriguez]

I only posted the diff here for discussion (just like I mentioned the alternative height: 0 approach), but as you noticed, nobody has proposed a PR to integrate it into master.

Some doubts remain however. What about the list of attachments as @mrcasals mentioned? What about the fact that the "button component" will appear after everything on mobile if we move the markup as @htmlboy noticed? Is that ok/intended?

[Crashillo]

Hey, @decidim/lot-mods you should take a look at this

[deivid-rodriguez]

To further clarify, in case someone is as confused as @Crashillo and I were:

I posted the diff as a possible solution for discussion, but I didn't check the design app (assuming it was syncronized). It turns out that my diff coincidentally matches the approved markup in the design app, so that (understandably) confused @Crashillo.

To sum up, this is a problem introduced when integrating the adhesions feature and should be fixed by proposing a PR to master with the mentioned diff.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. @carolromero & @xabier feel free to chime in.

[mrcasals]

This is still pending

/cc @decidim/lot-mods