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

Improve styling on the reverse dependencies page #3760

Conversation

dancristianb
Copy link
Contributor

@dancristianb dancristianb commented May 2, 2023

Resolves #3286

The reverse dependencies index page could use some styling touchups

  • The header should be properly aligned (width should be 100% and not 90%)
  • Search bar should be better aligned and possibly larger to fit the prompt text "Search reverse dependencies Gems". Maybe something that looks like the advanced search
  • If there are no reverse dependencies, do not provide a search

I think we should be OK with reusing the already existing home__search-wrap, home__search, and home__search__icon classes we use for the advanced to achieve what we want regarding the styles, let me know what you think 😀

Regarding the conditional render, I've tried sticking to the pattern and used @reverse_dependencies.present? to render/not render the search bar and results. Got a question here, should we maybe add a note something like "This gem has no reverse dependencies" just to be sure it's clear why we don't render anything? 😀

I've included a series of screenshots with the before and after below.

Before the updates

With reverse dependencies present

Screenshot 2023-05-02 at 23 53 45

Without reverse dependencies present

Screenshot 2023-05-02 at 23 53 51

After the updates

With reverse dependencies present

Screenshot 2023-05-02 at 23 53 04

Without reverse dependencies present

Screenshot 2023-05-02 at 23 53 12

@dancristianb dancristianb marked this pull request as ready for review May 2, 2023 21:22
@dancristianb
Copy link
Contributor Author

This one is now ready for review. Let me know what you think, thanks! 🙏

@simi
Copy link
Member

simi commented May 2, 2023

Hello! Thanks for the PR!

Would it be possible to keep the content inside the layout? I tried to explain with the red arrow on image. See on gem page nothing is crossing the arrow which is inline with top grey border.

image

On reverse deps page, some content escapes the layout.

image

@simi
Copy link
Member

simi commented May 2, 2023

Got a question here, should we maybe add a note something like "This gem has no reverse dependencies" just to be sure it's clear why we don't render anything?

Yes, that makes sense to me.

@codecov
Copy link

codecov bot commented May 2, 2023

Codecov Report

Merging #3760 (7d31826) into master (304b7dc) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 7d31826 differs from pull request most recent head 0fdf7cc. Consider uploading reports for the commit 0fdf7cc to get more accurate results

@@            Coverage Diff             @@
##           master    #3760      +/-   ##
==========================================
- Coverage   98.81%   98.81%   -0.01%     
==========================================
  Files         214      213       -1     
  Lines        5250     5229      -21     
==========================================
- Hits         5188     5167      -21     
  Misses         62       62              

see 6 files with indirect coverage changes

@dancristianb dancristianb marked this pull request as draft May 3, 2023 16:28
@dancristianb
Copy link
Contributor Author

Thanks @simi for the comments! 🙏 I updated the reverse dependencies page layout so it now matches the gems page.

Screenshot 2023-05-04 at 22 04 15

Screenshot 2023-05-04 at 22 04 26

And I've added in this This gem has no reverse dependencies. helper text right, hope that's OK.

Screenshot 2023-05-04 at 22 04 42

Let me know what you think, thanks!

@dancristianb dancristianb force-pushed the improve-styling-on-the-reverse-dependencies-page branch from 68bb7ea to 88e31a2 Compare May 4, 2023 19:11
@dancristianb dancristianb marked this pull request as ready for review May 4, 2023 19:11
@dancristianb dancristianb force-pushed the improve-styling-on-the-reverse-dependencies-page branch from 88e31a2 to 7d31826 Compare May 11, 2023 10:09
@simi simi self-requested a review May 15, 2023 23:02
@dancristianb dancristianb force-pushed the improve-styling-on-the-reverse-dependencies-page branch from 7d31826 to 18c1c26 Compare May 19, 2023 13:19
@simi simi force-pushed the improve-styling-on-the-reverse-dependencies-page branch from 18c1c26 to 0fdf7cc Compare May 31, 2023 20:26
@simi simi merged commit 711aeb9 into rubygems:master May 31, 2023
11 checks passed
@simi
Copy link
Member

simi commented May 31, 2023

Looks good. Thanks for the PR!

@dancristianb dancristianb deleted the improve-styling-on-the-reverse-dependencies-page branch March 5, 2024 15:27
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.

Improve styling on the reverse dependencies page
2 participants