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

Issue 679 split handout sections #692

Merged
merged 16 commits into from Oct 27, 2017

Conversation

Projects
None yet
3 participants
@railsstudent
Collaborator

railsstudent commented Oct 23, 2017

Fixes #679.

In the home of hangouts, divide hangouts into two sections, upcoming events and past events.
Similarly, hangouts are divided into upcoming events and past events in single study group

hangout_cards

single_study_group

@lpatmo Please let me know if i have done more than i should in single study group.
@distalx Please kindly code review my changes in study_group_hangouts.js. I am not sure if the implementation is efficient as the number of hangouts in a single study group increases considerably

Thanks.

@lpatmo lpatmo added the In Review label Oct 23, 2017

@lpatmo

This comment has been minimized.

Show comment
Hide comment
@lpatmo

lpatmo Oct 24, 2017

Member

Thank you @railsstudent! Will take a closer look tomorrow, but first thoughts that popped up to me:

  • make h3s white so that they stand out better against the blue
  • change first title to "Live and upcoming events", since there will be live events among them
  • do not show title at top of there are no scheduled events

For the last one, I was expecting {{#if hangouts}} to work, but apparently there is still an object even if there are no live/upcoming hangouts, so that is puzzling.

Will spend more time on this tomorrow night. Sorry!

Member

lpatmo commented Oct 24, 2017

Thank you @railsstudent! Will take a closer look tomorrow, but first thoughts that popped up to me:

  • make h3s white so that they stand out better against the blue
  • change first title to "Live and upcoming events", since there will be live events among them
  • do not show title at top of there are no scheduled events

For the last one, I was expecting {{#if hangouts}} to work, but apparently there is still an object even if there are no live/upcoming hangouts, so that is puzzling.

Will spend more time on this tomorrow night. Sorry!

@distalx

This comment has been minimized.

Show comment
Hide comment
@distalx

distalx Oct 24, 2017

Collaborator

Thanks for the PR! Here are some suggestions I hope you'll find helpful.

loadHangouts should contain all the hangout return Hangouts.find({}, {sort: {start: -1}});
we don’t have to filter hangouts at this stage, we will filter them inside template. So, we won’t be needing a loadCompletedHangouts function. you can remove that.

inside the template …

// this block will render current and upcoming hangouts
<div class="row margin-none" id="hangouts">
<h3 class="col-lg-12">{{_ "upcoming_events"}}</h3>
{{#each hangouts}}
	{{#unless isHangoutCompleted end}} // here we are filtering hangouts
		<div class="col-md-4">
		{{> hangoutCard}}
		</div>
	{{/unless}}
{{/each}}
</div>

//this block will render past hangots
<div class="row margin-none" id="past_hangouts">
<h3 class="col-lg-12">{{_ "past_events"}}</h3>
{{#each hangouts}}
	{{#if isHangoutCompleted end}} // here we are filtering hangouts
		<div class="col-md-4">
		{{> hangoutCard}}
		</div>
	{{/if}}
{{/each}}
</div>
Collaborator

distalx commented Oct 24, 2017

Thanks for the PR! Here are some suggestions I hope you'll find helpful.

loadHangouts should contain all the hangout return Hangouts.find({}, {sort: {start: -1}});
we don’t have to filter hangouts at this stage, we will filter them inside template. So, we won’t be needing a loadCompletedHangouts function. you can remove that.

inside the template …

// this block will render current and upcoming hangouts
<div class="row margin-none" id="hangouts">
<h3 class="col-lg-12">{{_ "upcoming_events"}}</h3>
{{#each hangouts}}
	{{#unless isHangoutCompleted end}} // here we are filtering hangouts
		<div class="col-md-4">
		{{> hangoutCard}}
		</div>
	{{/unless}}
{{/each}}
</div>

//this block will render past hangots
<div class="row margin-none" id="past_hangouts">
<h3 class="col-lg-12">{{_ "past_events"}}</h3>
{{#each hangouts}}
	{{#if isHangoutCompleted end}} // here we are filtering hangouts
		<div class="col-md-4">
		{{> hangoutCard}}
		</div>
	{{/if}}
{{/each}}
</div>
@railsstudent

This comment has been minimized.

Show comment
Hide comment
@railsstudent

railsstudent Oct 24, 2017

Collaborator

@distalx How can i determine upcoming or past events exist and display or hide header ?
I guess I will add two functions to call count function of MongoDB to return number of active events and past events respectively.
Then, call the functions to show or hide title accordingly.

Collaborator

railsstudent commented Oct 24, 2017

@distalx How can i determine upcoming or past events exist and display or hide header ?
I guess I will add two functions to call count function of MongoDB to return number of active events and past events respectively.
Then, call the functions to show or hide title accordingly.

@distalx

This comment has been minimized.

Show comment
Hide comment
@distalx

distalx Oct 25, 2017

Collaborator

@railsstudent , we can manage hangout count for each block with the help of reactive-variable.

Very roughly:

template.js

//inside onCreated

const now = new Date();
const hc = Hangouts.find({'end': { $gte : now }}, {sort: { start: 1 }}).count() || 0;
instance.liveOrUpComingHangout = new ReactiveVar(hc); //reactive variable

//inside helper 

liveOrUpComingHangout() {
 return Template.instance().liveOrUpComingHangout.get()
}

template.html

{{#if liveOrUpComingHangout}}
// this block will render current and upcoming hangouts
  [...]
{{/if}}
Collaborator

distalx commented Oct 25, 2017

@railsstudent , we can manage hangout count for each block with the help of reactive-variable.

Very roughly:

template.js

//inside onCreated

const now = new Date();
const hc = Hangouts.find({'end': { $gte : now }}, {sort: { start: 1 }}).count() || 0;
instance.liveOrUpComingHangout = new ReactiveVar(hc); //reactive variable

//inside helper 

liveOrUpComingHangout() {
 return Template.instance().liveOrUpComingHangout.get()
}

template.html

{{#if liveOrUpComingHangout}}
// this block will render current and upcoming hangouts
  [...]
{{/if}}
@railsstudent

This comment has been minimized.

Show comment
Hide comment
@railsstudent

railsstudent Oct 26, 2017

Collaborator

Done.
@lpatmo @distalx Please kindly review. thanks.

Collaborator

railsstudent commented Oct 26, 2017

Done.
@lpatmo @distalx Please kindly review. thanks.

@railsstudent railsstudent requested a review from anbuselvan Oct 26, 2017

@distalx

Great work @railsstudent! 👏 LGTM!

@lpatmo

This comment has been minimized.

Show comment
Hide comment
@lpatmo

lpatmo Oct 27, 2017

Member

LGTM too. Thank you!

Member

lpatmo commented Oct 27, 2017

LGTM too. Thank you!

@lpatmo lpatmo merged commit 1a197a1 into codebuddies:staging Oct 27, 2017

@lpatmo lpatmo added closed and removed In Review labels Oct 27, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment