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

Forum allow multiple categories for a thread #2083

Merged
merged 15 commits into from Jun 12, 2018

Conversation

Projects
None yet
4 participants
@scopeInfinity
Copy link
Member

scopeInfinity commented Jun 4, 2018

Additional Library

@scopeInfinity scopeInfinity requested a review from andrewaikens87 Jun 4, 2018

@scopeInfinity scopeInfinity force-pushed the forum_multiple_categories branch from 1aa7129 to 52cef29 Jun 7, 2018

@scopeInfinity scopeInfinity changed the title [WIP] Forum allow multiple categories for a thread Forum allow multiple categories for a thread Jun 8, 2018

@scopeInfinity

This comment has been minimized.

Copy link
Member Author

scopeInfinity commented Jun 8, 2018

UI for Selecting Categories on Create Thread Page
Screenshot

Category filter on View Thread page is expected to filter those thread who have provided category among its categories.

}else if(!$this->isValidCategory($category_id)){
$this->core->addErrorMessage("You must select a valid category. Please re-submit your thread.");
}else if(!$this->isValidCategories($categories_ids)){
$this->core->addErrorMessage("You must select valid categories. Please re-submit your thread.");

This comment has been minimized.

@andrewaikens87

andrewaikens87 Jun 9, 2018

Member

If the user has JavaScript disabled and selects no categories when posting the thread, it defaults to all categories for the thread. Seems it doesn't trigger this error message. To explain further, their isn't verification server sided that the user selected a category. If the user doesn't select a category with javascript disabled or by running these commands in the console

$("#create_thread_form").submit(function() { if($(this).find('.cat-selected').length == 0) { //alert("Atleast one category must be selected"); return true;//return false; } $(this).find('.cat-notselected *').prop("disabled","true"); });

document.getElementById("create_thread_form").submit();

it will bypass the client side check and submit the thread without having the user select a category. The behavior now is that all of the categories get selected by default in this case, the error checking exists on the server side but the data that is being sent to it is inconsistent (in this case specifically). If no category is selected and we submit, using the commands above, to the next page it should kick us back to the create thread page as no valid categories were selected.

This comment has been minimized.

@scopeInfinity

scopeInfinity Jun 10, 2018

Author Member

Thankyou for pointing out.
Fixed it as discussed on slack.

@andrewaikens87

This comment has been minimized.

Copy link
Member

andrewaikens87 commented Jun 9, 2018

The original design had the category drop down selection be able to filter on multiple categories (so similar to the way you have them selecting in the thread view you could add that into a dropdown). So for example a student could filter on both 'Question' and 'hw1' and see posts that only have both.

<script type="text/javascript">
$(function() {
$(".cat-buttons").click(function() {
console.log($(this));

This comment has been minimized.

@andrewaikens87

andrewaikens87 Jun 9, 2018

Member

Please remove console log

});
$("#create_thread_form").submit(function() {
if($(this).find('.cat-selected').length == 0) {
alert("Atleast one category must be selected");

This comment has been minimized.

@andrewaikens87

andrewaikens87 Jun 9, 2018

Member

Please fix typo and include period at end.

HTML;
for($i = 0; $i < count($categories); $i++){
$return .= <<<HTML
<a class="btn cat-buttons cat-notselected" style="color: green;border-color: green;">{$categories[$i]['category_desc']}

This comment has been minimized.

@andrewaikens87

andrewaikens87 Jun 9, 2018

Member

For interface colors refer to http://submitty.org/developer/interface_design_style_guide, please change to blue.

@andrewaikens87

This comment has been minimized.

Copy link
Member

andrewaikens87 commented Jun 9, 2018

When creating a new category it pops up saying that it has been created but the page has to be refreshed to use it with the new toggle buttons. Prior to the toggle buttons the dropdown list would be auto-populated when the new category was created. Can we have the new category auto-populate to a toggle button? We can discuss this more on Slack if it is unclear.

scopeInfinity and others added some commits Jun 9, 2018

@KevinMackenzie KevinMackenzie self-requested a review Jun 11, 2018

@KevinMackenzie

This comment has been minimized.

Copy link
Contributor

KevinMackenzie commented Jun 12, 2018

When adding a category, the green banner that gets displayed shows the url-encoded string, but it would probably be better to display the unencoded string in quotes instead.
image

The other changes I would request you seem to have already fixed based on the demonstration you gave in the meeting yesterday (and have already been pointed out by @andrewaikens87 )

@scopeInfinity

This comment has been minimized.

Copy link
Member Author

scopeInfinity commented Jun 12, 2018

Thanks @KevinMackenzie
I have pushed the changes for displaying category name in notifications in a better way.
However, the chances related to new category addition/deletion is made on #2135 PR(currently). So, I have pushed them there. I hope thats fine.

@KevinMackenzie

This comment has been minimized.

Copy link
Contributor

KevinMackenzie commented Jun 12, 2018

yea that's fine, it will be much less of a nightmare to merge that way.

@bmcutler bmcutler merged commit 0cf7f25 into master Jun 12, 2018

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment