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

Smyk / 2583 The button '+Додати ще дитину' should be disabled if the parent already has 20 children #2585

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

AndriiSmyk
Copy link
Contributor

#2583
Disabled 'Add another child' button when the parent already has 20 children.
Added information button with pop-up message when the parent already has 20 children.

…ildren.

Added information button with pop-up message when the parent already has 20 children.
@AndriiSmyk AndriiSmyk self-assigned this Jun 22, 2024
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can make a property that will show that the child limit is reached and reuse it inside the template instead of checking the number of children 5 times

@@ -99,6 +100,7 @@ export class CreateApplicationComponent implements OnInit, OnDestroy {
this.children$.pipe(filter(Boolean), takeUntil(this.destroy$)).subscribe((children: SearchResponse<Child[]>) => {
this.parentCard = children.entities.find((child: Child) => child.isParent);
this.children = children.entities.filter((child: Child) => !child.isParent);
this.has20Children = this.children.length >= 20;
Copy link
Contributor

Choose a reason for hiding this comment

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

A magic number in the property name is not the best option.
It is better to name it something like isChildLimitReached and create a constant MAX_CHILD or similar

AndriiSmyk and others added 2 commits July 3, 2024 09:31
…omparing to 20 with CHILDREN_AMOUNT_MAX constant
Copy link

sonarcloud bot commented Jul 3, 2024

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.

None yet

2 participants