Skip to content
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

Feature/redesign developer filters #409

Merged

Conversation

adrianvalenz
Copy link
Contributor

@adrianvalenz adrianvalenz commented May 3, 2022

Pull request checklist

  • Form is split into three components linked with the form attribute (desktop, mobile, and sort filter that ties both)
  • Write test to ensure elements appear on page and work together to filter correct developers
  • Style elements with css
  • Need to create a mobile ready component with it's own search mechanism. Should be one components vs the two working together for desktop

@joemasilotti
Copy link
Owner

joemasilotti commented May 3, 2022

Oooohh I'm excited for this!

I know you haven't added any styling yet, but I do want to call out one thing. When you do add styling, I'd love for this to fill the width of the screen (or at least be a little wider) like the screenshot from Tailwind UI below.

image


Let me know what you need from me and please keep me posted on progress!

@adrianvalenz
Copy link
Contributor Author

Are we using the Tailwind components exactly or just like it? Is there a style guide to follow. I'm thinking making it wider would created different width dimensions than what is there now on RailsDevs.com and would create inconsistency?

@joemasilotti
Copy link
Owner

Are we using the Tailwind components exactly or just like it?

Almost 100%. The only elements that aren't essentially copy-pasted are ones that don't exist like the developer card and profile pages.

Is there a style guide to follow.

No, no style guide.

I'm thinking making it wider would created different width dimensions than what is there now on RailsDevs.com and would create inconsistency?

That's fair. I think what I started with #348 is a good width that shouldn't feel too different from the rest of the site. But would love to hear your feedback if it starts to look weird!

@joemasilotti
Copy link
Owner

Hey again! What are next steps here? Sorry to be a pain but this feature is starting to become a priority as it is blocking a few other issues.

As always, let me know if you still have appetite to finish this up. Otherwise, happy to take it off your hands or pass on to someone else.

@adrianvalenz
Copy link
Contributor Author

adrianvalenz commented May 9, 2022 via email

@joemasilotti
Copy link
Owner

So I’m thinking since it is working on desktop, we can jump to making it work on mobile and styling it? At that point it should be ready to ship unless I’m missing something?

Sounds good to me! Do you have appetite to finish up mobile and styling?

The mobile menu should be a separate component that we can hide display on screen size, and it will make it easier to manage by separating those concerns.

Agreed - excited to see where you take this (if you want)!

After it is shipped, if it is ok with you, I’d like to continue refactoring it to abstract the repetitive logic to make it easier to add features, but that work doesn’t need to be completed for users to start using the new layout.

I would love that. There's definitely an abstraction that ties this more closely with DeveloperQuery to do exactly that. Especially for the upcoming filter in #297.

@adrianvalenz
Copy link
Contributor Author

Screen Shot 2022-05-29 at 11 16 09 PM

Screen Shot 2022-05-29 at 11 17 01 PM

Screen Shot 2022-05-29 at 11 16 51 PM

Screenshots of the new filter feature in action. It works pretty well.

  • I separated the original form from being one responsive file into two components, for both desktop and mobile.
  • The sort filter is in it's own (third component if you will) for ease of styling and it submits to the form via a form attribute
  • Originally I stripped all the css associated with the form because it had it's own opinions about responsive layouts, but since the design was different, it was easier to start with fresh html and new css.
  • I proceeded to add css for fonts, colors, buttons, checkboxes, etc before really getting to deep into grid/flex and layouts.
  • I needed to add some new locales for the new components. Ideally I'd like to abstract that as well and reduce duplication in those yaml files.
  • I added a couple more things not originally requested, such as search bar placeholder text, filter icon, and content.

Off the top of my head this is what I can think of. Any questions and comments I'm ready for! Thank you!

@adrianvalenz adrianvalenz marked this pull request as ready for review May 30, 2022 13:35
@joemasilotti
Copy link
Owner

joemasilotti commented May 31, 2022

Looking good so far! Here's my todo list as I work through the changes:

@joemasilotti
Copy link
Owner

Phew! Alright, I think we are good! Going to merge this one in now.

Thank you so much for the work on this @adrianvalenz! I appreciate it. This was a big one and I'm super excited to see it live.

@joemasilotti joemasilotti merged commit a20b045 into joemasilotti:main May 31, 2022
5 checks passed
@joemasilotti joemasilotti mentioned this pull request May 31, 2022
7 tasks
@adrianvalenz
Copy link
Contributor Author

adrianvalenz commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants