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

NgbPagination: Size 'md' missing from type #3816

Closed
softsimon opened this issue Jul 19, 2020 · 6 comments · Fixed by #4319
Closed

NgbPagination: Size 'md' missing from type #3816

softsimon opened this issue Jul 19, 2020 · 6 comments · Fixed by #4319

Comments

@softsimon
Copy link

Bug description:

Pagination.ts states:

 /**
     * The pagination display size.
     *
     * Bootstrap currently supports small and large sizes.
     */
    size: 'sm' | 'lg';

But the default size is actually 'md' which is a supported size.

So I am not allowed to set the [size] to 'md', without getting errors like this:

Type 'string' is not assignable to type '"sm" | "lg"'.

Solution

Add 'md' size to NgbPagination.size type

Versions of Angular, ng-bootstrap and Bootstrap:

Angular: 10
ng-bootstrap: 7
Bootstrap: 4.5

@maxokorokov
Copy link
Member

Pagination doesn't have 'md' sizing in Bootstrap → https://getbootstrap.com/docs/4.5/components/pagination/#sizing

However we should do the same we did for the modal (#3541) and convert size: 'sm' | 'lg'; to size: 'sm' | 'lg' | string;

@softsimon
Copy link
Author

Without a third option other than 'sm' | 'lg' there's no "default" (what I called md).

In my use case, I set the size to "default" (what I call md) or "sm" based on a condition. With only sm and lg as valid option for the [size] variable I can't set it to default. I guess | string will solve that though.

@maxokorokov
Copy link
Member

maxokorokov commented Aug 20, 2020

@softsimon thanks for the clarification, it's a valid point. So what you want is [size]="isLarge ? 'lg' : null". To avoid setting size at all.

To make it happen we have to either:

  • add this static ngAcceptInputType_size: string | null; to the NgbPagination
  • or use size: 'lg' | 'sm' | string | null;

@softsimon
Copy link
Author

I let you decide what makes more sense. At least now you know my specific use case 👍

[size]="isLarge ? 'lg' : null"

I believe this solution works for me right now, thanks!

@niharika412
Copy link
Contributor

Hi, I'm new here but can I give this a go?
Just to be sure, the change expected is size: 'sm' | 'lg'; to size: 'sm' | 'lg' | string; right?

@maxokorokov maxokorokov added this to the 9.1.2 milestone May 18, 2021
@maxokorokov
Copy link
Member

@niharika412, sure, please open a PR, this should be an easy one.

Example from a similar PR → 591426d

Just make sure to also add static ngAcceptInputType_size: string | null; to the NgbPagination. More on this if you're curious → #3623 (comment) and https://angular.io/guide/template-typecheck#input-setter-coercion

@fbasso fbasso modified the milestones: 9.1.3, 9.1.4 Jun 16, 2021
@maxokorokov maxokorokov modified the milestones: 12.0.3, 12.1.0 Apr 7, 2022
@maxokorokov maxokorokov modified the milestones: 12.1.0, 12.2.0 Apr 28, 2022
maxokorokov pushed a commit to maxokorokov/ng-bootstrap that referenced this issue May 3, 2022
@maxokorokov maxokorokov linked a pull request May 3, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants