Skip to content

Projects filters 3744#4280

Merged
roslynwythe merged 26 commits intohackforla:gh-pagesfrom
chrismenke45:Projects-Filters-3744
Apr 8, 2023
Merged

Projects filters 3744#4280
roslynwythe merged 26 commits intohackforla:gh-pagesfrom
chrismenke45:Projects-Filters-3744

Conversation

@chrismenke45
Copy link
Member

@chrismenke45 chrismenke45 commented Mar 24, 2023

Fixes #3744

What changes did you make and why did you make them ?

  • current-projects.html was changed to include proper elements for the js file to add filter elements to after DOM loaded
  • _base.scss was changed to have clip overflow instead of hidden to allow sticky filter sidebar to work THIS COULD POTENTIALLY AFFECT OTHER PAGES OF THE SITE (it changes a css value on the html&body elements), BUT I DON'T THINK IT HAS
  • _filter_dropdown.scss was updated to reflect new styling of filters
  • current-projects.js was updated to allow filters to be used as shown in Figma

Screenshots of Proposed Changes Of The Website

Visuals before changes are applied Desktop Layout Before Changed Mobile Layout Before Changed
Visuals after changes are applied

Desktop Layout After Changed

Mobile Layout After Changed

Mobile Layout After Changed (Filter Pop up)

@github-actions
Copy link

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b chrismenke45-Projects-Filters-3744 gh-pages
git pull https://github.com/chrismenke45/website.git Projects-Filters-3744

@github-actions github-actions bot added role: front end Tasks for front end developers Complexity: Large P-Feature: Projects page https://www.hackforla.org/projects/ size: 3pt Can be done in 13-18 hours labels Mar 24, 2023
@ExperimentsInHonesty
Copy link
Member

I left a note on the issue this is from #3744 (comment)

@chrismenke45 chrismenke45 added the Draft Issue is still in the process of being created label Mar 24, 2023
@github-actions github-actions bot removed the Draft Issue is still in the process of being created label Mar 24, 2023
@chrismenke45
Copy link
Member Author

Filter-Toolbar padding was reduced(by ~35%). I left a decent chunk there because I'm trying to make it at the level it should be with the search bar when it is implemented. If I should move it up further, please let me know :)

@bzzz-coding bzzz-coding self-requested a review April 3, 2023 05:27
@bzzz-coding
Copy link
Member

Availability: Tuesday and Thursday evenings
ETA: EOD 4/5/23

@ExperimentsInHonesty
Copy link
Member

Looks good.

@bzzz-coding
Copy link
Member

bzzz-coding commented Apr 5, 2023

@chrismenke45 Great job on adding the filter! The styles look good to me, and the tab focus worked on different screen sizes—thanks for ensuring good accessibility for the UX!

One thing I've noticed is that on medium-sized screens, the filter takes up half of the width (see screenshot for a screen width of 780px), and I wonder if that's the intended design. In my opinion, the filter shouldn't take more than 1/3 of the width, but that's subjective. I can't find a layout design in Figma that was made for medium-sized screens, nor have I found documentation that specifies the column ratio for the filter and cards. What are your thoughts on this?

Screenshot 2023-04-05 at 12 45 38 PM

@chrismenke45
Copy link
Member Author

Hi @bzzz-coding,
I reached out to @sijiapitts (the designer) on slack to see if she could shed some light on this.

@chrismenke45
Copy link
Member Author

Hi @bzzz-coding,
I got a response from sijia:
"But the issue is we only designed for desktop and mobile, not in between sizes. I’m thinking maybe we should use the mobile filter style for tablet size as well"
Do you think that'll work? I can make the changes.

@bzzz-coding
Copy link
Member

@chrismenke45 and @sijiapitts thank you for your input! Since we don't have a design made for medium-sized screens, I think whichever design the team thinks works best for this screen size would work.
@chrismenke45 Great job adding the filter! :)

@chrismenke45
Copy link
Member Author

Hi @bzzz-coding
I changed the breakpoint from $screen-tablet(768px) to $screen-desktop(960px). See below for the layout right before the new breakpoint at 960px. I will also be following up with @sijiapitts on slack to confirm this is an appropriate breakpoint. If not, I will add a custom breakpoint (as apposed to using our breakpoint values in scss variables).
Screen Shot 2023-04-06 at 7 41 41 AM

Copy link
Member

@roslynwythe roslynwythe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @chrismenke45 now in desktop view there is too much space between the filter and cards:

image

@bzzz-coding
Copy link
Member

bzzz-coding commented Apr 6, 2023

@chrismenke45 I was wondering if using aspect ratios or adjusting the column span would be a more responsive approach than setting a fixed pixel width. For instance, if the grid has three equal-width columns, the filter could occupy one column, while the two cards could span two columns each.

That being said, you have also put in a lot of effort towards this issue. Since there was no design provided for medium-sized screens, I am unsure whether adjusting layouts for such screens falls within the scope of this current issue. @roslynwythe, what are your thoughts on this?

@chrismenke45
Copy link
Member Author

@bzzz-coding & @roslynwythe,
Thank you for all the help on this issue.
In regards to the extra space between the filter and the cards on a large screen I have three possible solutions:

  1. Bitian's solution of setting the filters+project cards container to be 1fr for the filters and 2fr for the cards
  2. Add another column of project cards so they take up more space on the screen
  3. Add a max-width to the filters+project cards container so they are always next to each other and a wide screen will just have margins on the sides

Let me know what you think

@bzzz-coding
Copy link
Member

@chrismenke45 Thank you for proposing different solutions! I'm not an expert on frontend, but I do remember using something like autofit and setting a max value for grid-template-columns to control the maximum width of a column, as you suggested in solution 3. Or, maybe setting something like grid-template-columns: 1fr 2fr; alone would solve the problem?
Ultimately, I believe it's up to you to decide which CSS trick to use in order to change the layouts. Perhaps you can start with the one you're most familiar with and see if it works as intended?

@roslynwythe
Copy link
Member

roslynwythe commented Apr 6, 2023

@chrismenke45 @bzzz-coding I am not inclined to request further development on this PR for the sake of the medium size screens. I would prefer that we restore the desktop view to match the Figma design and leave the mid-size screen view as it was. You could create an ER to deal with the mid-sized screens. But I'm not the dev lead, so if either of you feel otherwise, please ask Justin to take a look.

@bzzz-coding
Copy link
Member

@chrismenke45 @roslynwythe I'm fine with the previous layout too and would approve the PR if you decide to change it back. Just let me know. Excellent job!

Copy link
Member

@bzzz-coding bzzz-coding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chrismenke45 Excellent job on adding the filter section to the page🌟!
Thank you for problem-solving and being flexible about different approaches to resolve an issue. 🙌

@chrismenke45
Copy link
Member Author

@bzzz-coding @roslynwythe
Thank you for all the help with my issue.
@roslynwythe If this last change was not what you had in mind, please let me know and I'd be happy to change it

@chrismenke45 chrismenke45 requested a review from roslynwythe April 6, 2023 23:38
Copy link
Member

@roslynwythe roslynwythe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @chrismenke45 on quite a challenging large issue. You've implemented a powerful live filtering feature, complete with dynamic filter lists that are very usable and look great. The PR is well formed and you responded to all requests thoughtfully and patiently. Thank you!

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

Labels

Complexity: Large P-Feature: Projects page https://www.hackforla.org/projects/ role: front end Tasks for front end developers size: 3pt Can be done in 13-18 hours

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix the dropdown menu design for projects page

4 participants