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

[AAE-25268] emit filter key when it's value increased or decreased #10186

Merged
merged 2 commits into from
Sep 12, 2024

Conversation

tomaszhanaj
Copy link
Contributor

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (check one with "x")

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation
  • Other... Please describe:

What is the current behaviour? (You can also link to an open issue here)

What is the new behaviour?
https://hyland.atlassian.net/browse/AAE-25268

Does this PR introduce a breaking change? (check one with "x")

  • Yes
  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

@@ -204,4 +212,15 @@ export class TaskFiltersCloudComponent extends BaseTaskFiltersCloudComponent imp
this.filters = [];
this.currentFilter = undefined;
}

checkIfFilterValuesHasBeenUpdated(filterKey: string, filterValue: number) {
if (!this.currentFiltersValues[filterKey]) {
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 sort out these condition to have a less branched method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!this.currentFiltersValues[filterKey]) {
if (this.currentFiltersValues[filterKey] !== filterValue) {
this.currentFiltersValues[filterKey] = filterValue; // you can improve this part to change the filter update only when needed
if (this.currentFiltersValues[filterKey]) {
this.updatedFilter.emit(filterKey);
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But with this change inner if will always be true so updatedFilter will be always emitted. But we do not want to do this when values are set for the first time.

image

@@ -204,4 +212,15 @@ export class TaskFiltersCloudComponent extends BaseTaskFiltersCloudComponent imp
this.filters = [];
this.currentFilter = undefined;
}

checkIfFilterValuesHasBeenUpdated(filterKey: string, filterValue: number) {
if (!this.currentFiltersValues[filterKey]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!this.currentFiltersValues[filterKey]) {
if (this.currentFiltersValues[filterKey] !== filterValue) {
this.currentFiltersValues[filterKey] = filterValue; // you can improve this part to change the filter update only when needed
if (this.currentFiltersValues[filterKey]) {
this.updatedFilter.emit(filterKey);
}
}

@tomaszhanaj tomaszhanaj force-pushed the feature/AAE-25268-refresh-button-for-tasks-v.2 branch from 0f98cf5 to e8499af Compare September 12, 2024 09:31
@@ -45,10 +45,6 @@ export abstract class BaseTaskFiltersCloudComponent implements OnDestroy {
@Output()
error: EventEmitter<any> = new EventEmitter<any>();

/** Emitted when filter is updated. */
@Output()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why removing this from here? this is in the base component that is being extended from the cloud one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

base component has only two outputs. Content-ee-apa is using TaskFiltersCloudComponent. Component TaskFiltersCloudComponent has also similar outputs like: filterSelected, filterClicked, filterCounterUpdated.

@VitoAlbano VitoAlbano force-pushed the feature/AAE-25268-refresh-button-for-tasks-v.2 branch from e8499af to c148d41 Compare September 12, 2024 15:54
Copy link

sonarcloud bot commented Sep 12, 2024

@VitoAlbano VitoAlbano merged commit cf9f4cc into develop Sep 12, 2024
17 checks passed
@VitoAlbano VitoAlbano deleted the feature/AAE-25268-refresh-button-for-tasks-v.2 branch September 12, 2024 16:17
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.

2 participants