search

CodeHubOrg / codehub-mentorships

CodeHub Mentorship platform - built with Laravel and React
3 stars 0 forks source link

Katja/marketingpage #89

Closed katjad closed 4 years ago

katjad commented 4 years ago

Making the site look a bit more compact: Standard header for all the pages that are there, and adding some text and stats (hardcoded so far) to Home page.

This PR also includes previous one, converting JS camelCase form field names to snakecase for DB columns.
See description here: https://github.com/CodeHubOrg/codehub-mentorships/pull/84

Plus also registration, server-side validation and old values being remembered by forms.

mentoring

fourstacks commented 4 years ago

@katjad - just been through this PR and I think there's a few areas we can make some changes to help reduce some complexity caused by the case-swapping. Rather than add loads of comments across the fairly large number of files in this PR I was thinking it might be better if we made a little time at some point in the next couple of weeks to hop on a pairing session (I could send you an invite so we can pair via Tuple). That way I can talk you through my thoughts on the Laravel code a bit so that you can ask any Qs as we go?

The TL;DR version is that I think we can create a middleware to recursively convert any keys that are passed in from the front end to camel case so that the rest of the app doesn't need to worry about key casing at all. When it comes to sending data to the front end I would leverage the Flexible Presenter package to define precisely what the keys should be.

If you're up for a pairing session then let me know on Slack when would suit you best and we'll work on that together maybe? We'd probably need at least 1 hour I'd have thought.

In the meantime I'll approve this PR so that we can work on a refactor in a new branch

katjad commented 4 years ago

Thanks @fourstacks, that would be great to talk through how we can simplify the case-converting, I will merge this in now and send you a slack message.