search

rubyforgood / pantry_scheduler

MIT License
7 stars 13 forks source link

Add Client Notes #108

Closed bolducp closed 7 years ago

bolducp commented 7 years ago

Can see & add notes from the dashboard Can see & add notes from the Client Form (in the dashboard and in the directory)

-- not quite ready for review; still an issue with adding notes from the Check In modal when accessed from the dashboard

pollygee commented 7 years ago

@bolducp is this now waiting for a review?

bolducp commented 7 years ago

@pollygee not quite, unfortunately :-( I ran into a bug that I've been having a hard time figuring out. I'd welcome help if anyone is able/willing. I'm sorry that it's taking so long. I should be able to spend some more time working on it once I'm back in town next weekend.

pollygee commented 7 years ago

OK no pressure at all! Def never apologize!! I just wanted to check in :) @jgaskins any chance you're able to take a look and offer some help?

jgaskins commented 7 years ago

Absolutely. @bolducp What's the bug? Is it in the form submission?

bolducp commented 7 years ago

@jgaskins, awesome. thanks! yeah, I was able to get it working so that you could add a new note from the directory page, but when I was trying to implement the same thing from the Check-In modal itself (I know it's not a great user experience to have a modal inside a modal anyway), it would work some times, but not consistently.

I ended up reverting some of the code, and I can't remember the exact state of what's pushed up now (that is, I can't remember exactly what's breaking at the moment), but if you would be able to glance that the current state of the PR and offer any advice, that'd be great. I've been out of town for the past two weeks, so it's been a bit since I've worked on it... I'm happy to also scrap what I've got and start from a new direction, if you think there's a more effective way to do it.

jgaskins commented 7 years ago

Sure, lemme give it a run and see what I can find.

bolducp commented 7 years ago

@jgaskins great. thank you!

jgaskins commented 7 years ago

@bolducp Buttons within a form have to opt out of form submission with type="button". I mean, you'd think you'd prefer to opt in with <button type="submit">, but nope. The DOM API is kinda confusing a lot of the time. :-)

The weird part is that it looked like it was just closing the modal because the app loads too quickly. I had to open the JS console to check that it was actually doing a full page reload.

I've gone ahead and made that change and it doesn't submit the form anymore. Should be good. :-)

bolducp commented 7 years ago

@jgaskins awesome. thanks so much! :-)

pollygee commented 7 years ago

@bolducp is this PR ready to go now? Or is there still more to do?

bolducp commented 7 years ago

@pollygee I think it should be good to go now, though there are definitely improvements that could be made. But after's Jamie's fix, I think the functionality should be working.

pollygee commented 7 years ago

Thanks. So OK if I merge this branch and then as we see improvements that can be made we'll add tickets to work on later?

bolducp commented 7 years ago

Yeah, that sounds good!

On Wed, Oct 11, 2017 at 7:46 PM, Polly Schandorf notifications@github.com wrote:

Thanks. So OK if I merge this branch and then as we see improvements that can be made we'll add tickets to work on later?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rubyforgood/pantry_scheduler/pull/108#issuecomment-335979840, or mute the thread https://github.com/notifications/unsubscribe-auth/ALK0rpL9QlHkPNYMNWGZwNuPIUmYm6S9ks5srVNugaJpZM4O1kAu .

-- www.paigebolduc.com