-
-
Notifications
You must be signed in to change notification settings - Fork 750
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 homepages 4593 #6747
Projects and homepages 4593 #6747
Conversation
…usability of browsing projects on home page and projects page
…projects-and-homepages-4593
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.
Note that CONTRIBUTING.md cannot previewed locally; rather it should be previewed at this URL:
|
Availability: weekdays |
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.
@Anahisv23 Thank you for working on this issue. Merge branch is setup correctly. Code changes show expected output. Website works well on my local machine. Well done. Approved!
Review ETA: 5/7/24 |
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 @Anahisv23, great work on the issue! Your branches are correct, there is a linked issue, and the correct css styling has been added to achieve the goals of the issue. Overall, very awesome job!
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.
Hi @Anahisv23,
Great progress on this issue. The filter section and projects scroll independently, and the search bar remains pinned to the top of the projects list.
However, during testing, I noticed some behavior that needs to be addressed:
- The content height appears to have been reduced, limiting the viewable area. The content height should fill the viewable area. The filter/project section should scroll to the top of the page and become sticky when it reaches the top, just below the navigation bar, to increase the screen real estate for viewing projects.
- The project list should scroll with the rest of the page, similar to the REI example provided in the issue, where the list shares the same vertical scroll as the page. This will also avoid competing vertical scrolls on horizontally shorter browser windows.
- The filter section should not shrink horizontally or have horizontal scroll.
- On screen width 768px and below, when opening filters, there is a gap between filters and action buttons, allowing the projects list to be visible and scrollable underneath.
Please make the necessary adjustments to address these issues. Test the changes on both the Projects page and the Home page.
Hey @jphamtv thanks for the feedback! I'll look into making these adjustments this week. My availability for this week is from 11am - 5pm. |
Update: I worked on addressing some of the feedback you @jphamtv mentioned but I had a very busy week with interviews so I'm looking of getting this done next week! |
fbc6e30
Hi @jphamtv, I went ahead and made changes to my approach to address the concerns you had. Let me know what you think! What my new changes address
|
Thank you @Anahisv23 for making the requested changes. Following are the behavior that I noticed.
@jphamtv Please share your thoughts on these observations. |
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.
Hi @Anahisv23,
Great work on the updates, the following are working well:
- The project section is perfectly implemented for the desktop.
- The filter and project sections scroll to the top of the page, increasing the viewable area.
- The filter section doesn't have horizontal scroll.
- No competing scrolls, resulting in much better functionality.
In addition to the feedback from @Thinking-Panda, please address the following:
- Revert the filter section's horizontal shrinking on desktop view to match the current website's behavior, where only the project cards shrink instead of the filter section.
- Ensure the filter's content height fills the viewable area on desktop view (maybe replace 750px with 100vh and see if that works?).
- On screen width 768px and below, when opening filters, eliminate the gap between the filter and action buttons to prevent projects from being visible underneath.
Thanks for your efforts on this. Reach out if you have any questions.
Hi @Thinking-Panda and @jphamtv thanks for the feedback and visual feedback on this issue. I'll look into this tomorrow or next week. I'm currently interviewing for a company so I have been a bit swamped with work. |
Hi @jphamtv I've been working on this and I think I finally solved the scrolling inconsistencies! Let me know if you are experiencing them on your end. |
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.
@Anahisv23 The way the file is removed from the commit might remove the file from the repo once the PR is merged. I have asked for help from merge team on further steps to be taken. Thank you for your patience and understanding.
Hi @Thinking-Panda thanks for asking the merge team about this! My bad I didn't mean to remove it entirely. Would you like me to pull the latest changes from gh-pages, create a new food-oasis.md file on my branch, and just copy and past the code from food-oasis.md on gh-pages onto my branch? |
Hi @Anahisv23! Yes, that is likely the least complicated way to resolve the issue. You can go ahead and do that. |
Hi @LRenDO! I went ahead and made a new food-oasis file and added the most recent changes to food oasis today. Let me know if that fixes this issue. |
…projects-and-homepages-4593 merge gh-pages into projects-and-homepages-4593
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.
Hi @jphamtv I've been working on this and I think I finally solved the scrolling inconsistencies! Let me know if you are experiencing them on your end.
@Anahisv23 - The nested scrolling issue is now resolved - no more conflicts on my end. Excellent work figuring out the scrolling problems and implementing this feature. Thanks for all your effort addressing the feedback.
Hey @jphamtv awesome! I'm glad the scrolling issues are resolved. Thank you for your feedback and help throughout this ticket. I appreciate y'all! Thank you :) I believe now we just need @Thinking-Panda and @mSharifHub to review this and approve it to get this merged. |
@Anahisv23 As noticed earlier, the project list scrolls when the pointer is still in the filter section on my local machine. If other reviewers do not notice that behavior, I can go ahead and approve this PR. I will wait for the other reviewer's comments. Thank you and appreciate your patience. |
… is on filter toolbar
…projects-and-homepages-4593 Merge branch 'gh-pages' of https://github.com/hackforla/website into projects-and-homepages-4593
Hi @Thinking-Panda I just noticed this problem on my end too. Thank you for bringing it up! I did some research and applied the |
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.
@Anahisv23 Excellent work. All scrolling works fine on my end. Thank you for your patience and perseverance.
@Thinking-Panda Thank you and thank you so much for your help throughout this ticket! I'm glad everything looks good on your end. I believe now we just need @jphamtv and @mSharifHub to review this and approve it to get this merged. |
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.
Hi @Anahisv23 and @Thinking-Panda - I tested the scrolling behavior @Thinking-Panda mentioned and it works as expected on my end as well.
Hi @jphamtv. Awesome, I'm glad its working as expected! Now we just need @mSharifHub to take a look and approve. |
Merging this PR b/c it has three approvals |
* modified styling for filter, searchbar, and projects list to improve usability of browsing projects on home page and projects page * addressed concerns on this issue * Update _dropdown_filters.scss line 19 * Update _main.scss by removing height styling * pinned search bar on mobile * updated styling on search bar to prevent it from pinning on mobile * updated filtersDiv, removed gap in between projects list and filters, and changed height of filter-toolbar * removed food-oasis.md file from commit * modified max height on dropdown show all css rule to fix scroll inconsistencies * addressing scrolling inconsistencies * created a new food-oasis.md file * added overscroll-behavior to prevent project list scroll when pointer is on filter toolbar
Fixes #4593
What changes did you make?
Additions to CSS files
Modifications to existing CSS files
Note:
Why did you make the changes (we will use this info to test)?
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