JoshuaStorm / meetable

A free web application for simplifying the process of scheduling individual and group meetings.
https://meetable-us.herokuapp.com/
4 stars 3 forks source link

UI issue fix with start before/after #275

Closed shayanmonshizadeh closed 7 years ago

shayanmonshizadeh commented 7 years ago

closes #258

Assumed backend was handled and simply changed the front end.

davidmelvin commented 7 years ago

this accepts "2pm - 10am", should it? i'm still kinda confused about how you guys wanted to change it

JoshuaStorm commented 7 years ago

Yes. We should accept any time such that Begin != End

The logic is you're really creating a range of times when you want to meet: 2pm - 10am means you want to meet any time that is the complement of (10am-2pm).

It's a fairly unusual case that few users would use, but I see no reason we should not support it

shayanmonshizadeh commented 7 years ago

"2pm - 10am" means that the user wants no meetings before 2pm and no meetings after 10am. That means if I have classes everyday from 10am to 2pm, which is reasonable, I never want to meet during this time. Thus I set it to 2pm-10am. Can you also test by sending out an invite and confirming that it does what I just said

JoshuaStorm commented 7 years ago

Also this passed all my tests!

davidmelvin commented 7 years ago

Settings meetRange to "8pm - 7pm" changed the suggested times in the screenshot to say I wasn't meetable at all. Is that the desired behavior? I think not? Although I think this is not an error in the PR it's a backend error

screen shot 2017-05-11 at 11 20 11 pm
shayanmonshizadeh commented 7 years ago

Shit. @JoshuaStorm this is a backend issue? If there are no times before 8pm and no time after 7pm that means no time at all. How should we error check for this kind of behavior?

davidmelvin commented 7 years ago

i'm 90% sure we can merge this PR tho

shayanmonshizadeh commented 7 years ago

No. Veto on the merge. That's actually a bad issue that should be addressed in this PR

shayanmonshizadeh commented 7 years ago

Hold on actually I'm wrong. No time before 8pm and after 7pm means you don't want to meet from 7-8 on any given day. So I'm not sure why that's not working. Still a backend issue unless I messed something up by setting the dates the way I did

JoshuaStorm commented 7 years ago

Uh oh, that's not good. I'll have a look right now. Good catch @davidmelvin

JoshuaStorm commented 7 years ago

Sorry about that, should be good now. Give it a few test runs of course tho

cli1209 commented 7 years ago

This looks good to me

shayanmonshizadeh commented 7 years ago

@JoshuaStorm still getting no meetable times when I set it to 8PM - 7PM

JoshuaStorm commented 7 years ago

@supert165 You sure you pulled the changes? Lemme triple check

shayanmonshizadeh commented 7 years ago

@JoshuaStorm ahh my bad. Pulled and seems to be working

JoshuaStorm commented 7 years ago

Imma go ahead and merge this 👍