-
-
Notifications
You must be signed in to change notification settings - Fork 724
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
"Projects" and "Home Page" Edits | pins project filter to top of project list, un-sticky's the Filters on scroll, & centers components #4694
Conversation
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.
|
Availability: 5/18 evening, 5/19 |
Review ETA: 5/18/2023 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @agosmou great adding the filter on the div class to keep the code very well read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @agosmou great job adjusting the projects filter: removing the sticky styling, pinning to the top of the project list, and centering on the page for the Projects and homepages.
When I view in Docker, I am noticing the following:
- In all views, the 'Applied Filters' notice no longer appears. I am not arguing whether or not the 'Applied Filter' notice is useful or worth keeping, only that removing this notice is beyond the scope of the issue.
- In tablet and desktop views, the red applied filter tags have shifted from the Filters column and now are above the project cards. I believe that keeping the red applied filter tags in the current location is more intuitive for the user.
- In the tablet/ 767 px view, the Filters column width compresses and looks 'smushed' in comparison to the way it looks currently in the same view. I feel that maintaining the width of the Filters column is preferable visually.
- Lastly, I am not entirely clear about how the /projects-check page is used currently or whether there is even a link to this page from the current website, however the edits made with the PR also change the layout of this page- both the 'Current Projects' header and the project cards themselves become off-center.
Thanks for your work so far!
Hey @t-will-gillis - Thank you for your thoroughness. See Below:
I feel like this was implied as I had to match REI's page (where their filter's don't have that text) per Bonnie's comment on the Emergent Request
I had to match REI/Amazon's page per Bonnie's comment on the Emergent Request
Nice catch - Those properties are used in a lot of places throughout the whole website. I went ahead and fixed this in the CSS just in case (not sure this page is used). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @agosmou – Thanks for the clarification/ explanation about the placement of the red applied filter tags. I can see why you did what you did when I look at the REI page. Also, when the filter tags are at the top of the project cards, there is not really a good location for the ‘Applied Filters’ verbiage and since it does not add to clarity, I agree with you that the text can/should be removed.
![rei_comp](https://private-user-images.githubusercontent.com/40799239/239723034-579b2be6-cc5c-4f92-857c-6d5d76317c5b.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjEzNDMzMTAsIm5iZiI6MTcyMTM0MzAxMCwicGF0aCI6Ii80MDc5OTIzOS8yMzk3MjMwMzQtNTc5YjJiZTYtY2M1Yy00ZjkyLTg1N2MtNmQ1ZDc2MzE3YzViLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA3MTglMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwNzE4VDIyNTAxMFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTIxZTg3MTZmYWQ3YzhkNmI5OTZjZjc2YWVhZGI2OTIxYTY4YzUxMWU0Yzg3NGFhMGYxYTNlOTQxNTIzOTQwZjEmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0._yLN5Ta0ZPsxQelMx92pL44-RFR6uDpz1twO_N4anc8)
Thanks also for changing the min width of the Filters column (I am just noticing the responsive design changes from 1 to 2 to 3 columns going from 766 to 767 to 768 on the current site) and for adjusting the /projects-check page. Both look good now.
Great job working through this issue!
Fixes #4593
What changes did you make and why did you make them ?
Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)
Visuals before changes are applied
Visuals after changes are applied