-
Notifications
You must be signed in to change notification settings - Fork 174
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
fix(pagination): styles #501
Conversation
* cursor text is the default when routerLink has receives no link
@@ -53,7 +58,7 @@ import { HlmPaginationDirective } from './hlm-pagination.directive'; | |||
|
|||
@if (showEdges() && !isLastPageActive()) { | |||
<li hlmPaginationItem (click)="goToNext()"> | |||
<hlm-pagination-next /> | |||
<hlm-pagination-next class="cursor-pointer" /> |
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.
question: What do you think about adding this class conditionally based on link().trim().length === 0
(disabled) state of hlm-pagination-next
/ hlm-pagination-previous
?
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.
Thats an even better idea! I will see if I can even add it to hlm-pagination-link.
@@ -47,6 +47,7 @@ export class HlmPaginationLinkDirective { | |||
protected readonly _computedClass = computed(() => | |||
hlm( | |||
paginationLinkVariants(), | |||
this.link() === undefined ? 'cursor-pointer' : '', |
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.
@ajitzero I apply the cursor-pointer
in the pagination link. This also applies the style to pagination next/previous as they use the pagination link directive as well.
The routerLink
type can be string | any[] | UrlTree | null | undefined
, maybe its sufficient to only apply the styles when no link is applied === undefined
. What do you think?
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.
Yes, that makes sense! Thanks!
PR Checklist
Please check if your PR fulfills the following requirements:
guidelines: https://github.com/goetzrobin/spartan/blob/main/CONTRIBUTING.md#-commit-message-guidelines
PR Type
What kind of change does this PR introduce?
Which package are you modifying?
What is the current behavior?
Pagination previous had the wrong
pr-2.5
style applied.Advanced Pagination hover styles showed the cursor text by default, because
RouterLink
is used as hostDirective forhlmPaginationLink
and it does not receive alink
.What is the new behavior?
Update pagination previous styles to
pl-2.5
.Add
cursor-pointer
to advanced pagination tohlmPaginationLink
and previous/next buttons.Does this PR introduce a breaking change?
Breaking change because styles change and need to copied over
Other information