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
Show ideas one status at a time #164
base: master
Are you sure you want to change the base?
Conversation
ping @ivandiazwm |
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.
Generally I would like to see this as configurable option to not change the behaviour for current installs.
$page = (int) $page; | ||
$category = (int) $category; | ||
$max = $this->getSetting('max_results'); | ||
$from = ($page - 1) * $max; | ||
$query = "SELECT * FROM ideas WHERE categoryid='$category' AND status !='new' ORDER BY "; | ||
$query = "SELECT * FROM ideas WHERE categoryid='$category' AND status = '$status' ORDER BY "; |
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 seems the logic is flipped here. Why?
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 PR proposes a mechanism for filtering ideas by status. Here is the db query for fetching our data. new
is still filtered out, but in the controller side. IMO, it's not the storage responsibility to assert that a status is never accessible.
</ul> | ||
</div> | ||
|
||
<div style="color:#34495E"><small><?php echo $category->description; ?></small></div> |
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 try to not use any inline styles.
Oops, I missed that. |
@prytoegrian The idea is definitely good and helpful when we have too many ideas to be filtered. Please make it a configurable option and let the rest of your changes run accordingly. Let us know once done, we will review, test and merge this PR asap. |
I felt very frustrated by the ideas current display so I improved it a little.
IMO, when an user want to see ideas, he doesn't really want to see all ideas. Only the current pipeline is important, ie. considered ideas. That's why the current status filter is set on 'considered'.
But, it might be useful to list ideas of another type, but only one status at a time. To support this UX I designed a select allowing to choose a status in header's list. Ideas obviously remain sorted by vote DESC.
For sure, I may be wrong and in that case, don't hesitate to close this PR.
Thanks !