Skip to content

Fix pagination styling (ARC-178) - #97

Merged
ssciolla merged 3 commits into
mainfrom
arc-178-pagination-styling
Sep 3, 2026
Merged

Fix pagination styling (ARC-178)#97
ssciolla merged 3 commits into
mainfrom
arc-178-pagination-styling

Conversation

@ssciolla

@ssciolla ssciolla commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

What it looks like now:

Screenshot 2026-09-03 at 10 49 05 AM

@ssciolla
ssciolla requested a review from rshiggin September 3, 2026 14:49
@ssciolla ssciolla added the bug Something isn't working label Sep 3, 2026
@ssciolla ssciolla changed the title Fix pagination styling Fix pagination styling (ARC-178) Sep 3, 2026
Comment thread app/assets/stylesheets/_base.scss Outdated
/* Pagination */

.page-link {
color: var.$color-blue-400 !important;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It looks to me like !important is overriding our style in search_results.scss

.page-link {
  color: var.$color-teal-400;
}

Prod is also using a darker initial color: color: var(--color-indigo-500); See below. Prod is using indigo on active, too.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I moved the styles over to _search_results.scss: b0bb1a5

I think I prefer the color-blue-400; rather than adding indigo to the palette. What do you think?

Comment thread app/assets/stylesheets/_base.scss Outdated

.page-link:hover {
color: var.$color-blue-400 !important;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

hover is generally used to change an active item for accessibility. In this instance, hover is identical to the non-hover state.

We are inheriting some slight shading on hover from bootstrap, but usually two changes are necessary to be accessible enough.

Prod hover, for example, 1) shades the page number box more and 2) removes the initial underline on the number itself.

.page-link:hover {
    z-index: 2;
    color: #0056b3;
    text-decoration: none;
    background-color: #e9ecef;
    border-color: #dee2e6;
}

!important does seem necessary for the hover value to override bootstrap.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think I've addressed this with my changes. For non-active items, the hover background is the same color as prod. The text color is color-blue-400, which I prefer over the indigo as stated above. There is underlining when no hovering is happening, and no text decoration when hovered on. For active items, there is (or should) be no hover behavior and no text decoration, which seems consistent with prod. There may be some slight differences in the gray border on non-active items, but I'm not certain. b0bb1a5 ddf5bb9

Comment thread app/assets/stylesheets/_base.scss Outdated
background-color: var.$color-blue-400 !important;
border-color: var.$color-blue-400 !important;
color: var.$color-background-primary !important;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Again, !important looks like it's overriding our own _search_results.scss

.page-item.active .page-link {
  background-color: var.$color-background-primary;
  border-color: var.$color-background-primary;
}

And prod is using

.page-item.active .page-link {
    background-color: var(--color-indigo-500);
    border-color: var(--color-indigo-500);
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Consolidated. Using blue over indigo (let me know about that above). b0bb1a5

@ssciolla
ssciolla requested a review from rshiggin September 3, 2026 18:43
@ssciolla
ssciolla merged commit 143c184 into main Sep 3, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants