-
-
Notifications
You must be signed in to change notification settings - Fork 392
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FIX] Do not link when there are no endorsements. #3531
[FIX] Do not link when there are no endorsements. #3531
Conversation
@@ -1,4 +1,4 @@ | |||
<%= link_to "#list-of-endorsements", id: "proposal-#{proposal.id}-endorsements-count", class: "button small compact light button--sc button--shadow #{fully_endorsed ? 'success' : 'secondary'}" do %> | |||
<%= link_to "#{proposal.proposal_endorsements_count.positive? ? "#list-of-endorsements" : "javascript:;"}", id: "proposal-#{proposal.id}-endorsements-count", class: "button small compact light button--sc button--shadow #{fully_endorsed ? 'success' : 'secondary'}" do %> |
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.
Maybe we should use link_to_if
here or not render the link at all?
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 can use a variable if you don't like this inliner:
<% link_href= proposal.proposal_endorsements_count.positive? ? "#list-of-endorsements" : "javascript:;" %>
+<%= link_to link_href, id: "proposal-#{proposal.id}-endorsements-count", class: "button small compact light button--sc button--shadow #{fully_endorsed ? 'success' : 'secondary'}" do %>
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.
The problem is not the inline ternary operator, but the link to javascript:;
, which looks weird to me
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.
Indeed, but it is solution to the problem that avoids having to hack the CSSs.
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.
@tramuntanal You don't need to hack the CSS here. You can apply those classes to a div
and it will look exactly the same 😄
3c44b67
to
6a62957
Compare
It seems that coverage checks are stalled @decidim/lot-core |
This PR is ready to be merged @decidim/lot-core :) |
b46003f
to
7b8a672
Compare
🎩 What? Why?
There is a strange behaviour when clicking on icon next to endorse button.
📌 Related Issues
📋 Subtasks
CHANGELOG
entry