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

Projects and homepages 4593 #6747

Merged

Conversation

Anahisv23
Copy link
Member

Fixes #4593

What changes did you make?

Additions to CSS files

  • Added the form#search-bar css, line 13 in _search-bar.scss file. Added position sticky to keep search bar pinned
  • Added the .projects-and-filters css, line 40 in _dropdown_filters.scss. Added overflow-y to allow independent scrolling of projects

Modifications to existing CSS files

  • Modified the .filter-toolbar css, line 32 in _dropdown_filters.scss. Added overflow-y to allow independent scrolling of filters list
  • Modified the .filtersDiv css, line 46 in _dropdown_filters.scss. Added position sticky to keep filters section pinned

Note:

  • Modifying the _dropdown_filters.scss and _search-bar.scss files applies css changes to both the projects page and home pages user interface.

Why did you make the changes (we will use this info to test)?

  • To improve the usability of the filters, search and project display so that it is easy for people to navigate the project list.

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

Visuals before changes are applied

image

Visuals after changes are applied

image

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 Anahisv23-projects-and-homepages-4593 gh-pages
git pull https://github.com/Anahisv23/website.git projects-and-homepages-4593

Note that CONTRIBUTING.md cannot previewed locally; rather it should be previewed at this URL:

https://github.com/Anahisv23/website/blob/projects-and-homepages-4593/CONTRIBUTING.md  

@github-actions github-actions bot added role: front end Tasks for front end developers Complexity: Large P-Feature: Home page https://www.hackforla.org/ P-Feature: Projects page https://www.hackforla.org/projects/ size: 1pt Can be done in 4-6 hours labels Apr 24, 2024
@Thinking-Panda Thinking-Panda self-requested a review April 30, 2024 19:27
@Thinking-Panda
Copy link
Member

Availability: weekdays
Review ETA: 5/1/24 EOD

Thinking-Panda
Thinking-Panda previously approved these changes May 1, 2024
Copy link
Member

@Thinking-Panda Thinking-Panda left a 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!

@irais-valenzuela irais-valenzuela self-requested a review May 7, 2024 20:15
@irais-valenzuela
Copy link
Member

irais-valenzuela commented May 7, 2024

Review ETA: 5/7/24
Availability: 5/7/24

Copy link
Member

@irais-valenzuela irais-valenzuela left a 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!

Copy link
Member

@jphamtv jphamtv left a 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:

  1. 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.
  2. 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.
  3. The filter section should not shrink horizontally or have horizontal scroll.
  4. 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.

@Anahisv23
Copy link
Member Author

Hey @jphamtv thanks for the feedback! I'll look into making these adjustments this week. My availability for this week is from 11am - 5pm.

@Anahisv23
Copy link
Member Author

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!

@Anahisv23
Copy link
Member Author

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

  1. Content section pins just below navigation
  2. Removed scrolling from .project-and-filters so projects lists scrolls with whole page
  3. Fixed width on .filter-toolbar and added overflow-x to remove horizontal scroll
  4. Fixed gap in between filters and action buttons by adding max height property to .filter-toolbar.show-filters on mobile media query

@Thinking-Panda
Copy link
Member

Thank you @Anahisv23 for making the requested changes. Following are the behavior that I noticed.

  1. In mobile viewport, the size of the filter-toolbar has changed.

    Before Change:
    MobileFilterBeforePR6747
    After Change:
    MobileFilterAfter-PR6747

  2. In mobile viewport, the filter and the search bar are pinned to the top and causing an overlap.

Large mobile PR 6747

  1. In desktop viewport, the project list scrolls when the pointer is still in the filter-toolbar.

Recording-PR6747-ezgif com-video-to-gif-converter

@jphamtv Please share your thoughts on these observations.

Copy link
Member

@jphamtv jphamtv left a 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.

@Anahisv23
Copy link
Member Author

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.

@irais-valenzuela irais-valenzuela removed their request for review May 23, 2024 21:57
@Anahisv23
Copy link
Member Author

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.

mSharifHub
mSharifHub previously approved these changes Jul 11, 2024
Copy link
Member

@Thinking-Panda Thinking-Panda left a 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.

@Anahisv23
Copy link
Member Author

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?

@LRenDO
Copy link
Member

LRenDO commented Jul 24, 2024

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.

@Anahisv23
Copy link
Member Author

Anahisv23 commented Jul 26, 2024

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.

jphamtv
jphamtv previously approved these changes Aug 1, 2024
Copy link
Member

@jphamtv jphamtv left a 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.

@Anahisv23
Copy link
Member Author

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.

@Thinking-Panda
Copy link
Member

@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.

@Anahisv23
Copy link
Member Author

Anahisv23 commented Aug 3, 2024

@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.

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 overscroll-behavior: contain; css property to the filter-toolbar class. I tested it on my end and it no longer causes the project list to scroll when hovered over the filter toolbar element. Let me know how it is on your end.

@Anahisv23 Anahisv23 requested a review from jphamtv August 3, 2024 01:01
Copy link
Member

@Thinking-Panda Thinking-Panda left a 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.

@Anahisv23
Copy link
Member Author

@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.

Copy link
Member

@jphamtv jphamtv left a 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.

@Anahisv23
Copy link
Member Author

Hi @jphamtv. Awesome, I'm glad its working as expected! Now we just need @mSharifHub to take a look and approve.

@t-will-gillis
Copy link
Member

Merging this PR b/c it has three approvals

@t-will-gillis t-will-gillis merged commit 664c768 into hackforla:gh-pages Aug 12, 2024
3 checks passed
del9ra pushed a commit to del9ra/website that referenced this pull request Aug 16, 2024
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complexity: Large P-Feature: Home page https://www.hackforla.org/ P-Feature: Projects page https://www.hackforla.org/projects/ role: front end Tasks for front end developers size: 1pt Can be done in 4-6 hours
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Projects and Home Pages: Pin Projects Filter to the Top of the Projects List
7 participants