-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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: show "more call for speaker" button when appropriate #3610
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.
Works fine
app/templates/index.hbs
Outdated
<div class="ui centered grid"> | ||
<div class="row"> | ||
{{#link-to "explore" (query-params cfs='open') class='ui blue button'}}{{t 'Show more calls for speakers'}}{{/link-to}} | ||
{{#if (gt callForSpeakersEvents 3)}} |
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.
It's not the correct way callForSpeakersEvents
returns the queried events not there number. Also, the comparison works here due to the Ember Arith Helper
. I think a new property CFSeventsnumber
should be used for comparison, I am also not sure if adding new property will be optimal or not. Hence, I will wait for the review of other mentors.
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.
@kushthedude i understand what you are trying to say . So i changed the comparison here from {{#if (gt callForSpeakersEvents 3)}} to {{#if (gt callForSpeakersEvents.length 3)}}
now here is no reason to say it false in my opinion.
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.
Please check it .
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.
@uds5501 can you please check it again that it is also working fine ?
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 changed the code ,as @kushthedude said .The previous was working fine but this gives a better understanding IMO.
@kushthedude can you please check it again , if it working fine ? |
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.
LGTM
Thanks a lot for this PR. Please help to get some important issues out of the way in this project. We have labeled these issues here: https://github.com/fossasia/open-event-frontend/issues?q=is%3Aissue+is%3Aopen+label%3A%22Priority%3A+High%22 Our goal is to focus on these issues. |
Ok @mariobehling i will trt to solve these issues . |
Fixes #3309
![Screenshot from 2019-11-08 09-44-02](https://user-images.githubusercontent.com/56407566/68449313-9f146b00-020c-11ea-9ee9-1ec4a1edd147.png)
changes it propose
when there are less than or equal to three the "show more call for speaker " button will not be visible .
Checklist
development
branch.