Enhancement: Filter-type-group component created #1210

verena-ifx · 2024-05-02T20:47:57Z

By creating this pull request you agree to the terms in CONTRIBUTING.md.
https://github.com/Infineon/.github/blob/master/CONTRIBUTING.md
--- DO NOT DELETE ANYTHING ABOVE THIS LINE ---

CONTRIBUTING.md also tells you what to expect in the PR process.

Description
Filter Type Group component + related sub components added
To be used inside the advanced table

📦 Published PR as canary version: 21.10.0--canary.1210.9f5143c2fbd5e614f57580d3b34852bfe6ffa12e.0

✨ Test out this PR locally via:

npm install @infineon/infineon-design-system-stencil@21.10.0--canary.1210.9f5143c2fbd5e614f57580d3b34852bfe6ffa12e.0
# or 
yarn add @infineon/infineon-design-system-stencil@21.10.0--canary.1210.9f5143c2fbd5e614f57580d3b34852bfe6ffa12e.0

…ne component

github-actions · 2024-05-02T20:50:35Z

PR Preview Action v1.4.7
🚀 Deployed preview to https://Infineon.github.io/infineon-design-system-stencil/pr-preview-react-example/pr-1210/
on branch `gh-pages` at 2024-05-27 10:08 UTC

github-actions · 2024-05-02T20:50:36Z

--STORYBOOK-PREVIEW--

PR Preview URL for Storybook: https://Infineon.github.io/infineon-design-system-stencil/pr-preview/pr-1210/

github-actions · 2024-05-02T20:55:28Z

--EXAMPLE-APPS-PREVIEW--
PR Preview URLs:

Vue example: https://Infineon.github.io/infineon-design-system-stencil/pr-preview-vue-example/pr-1210/
React example: https://Infineon.github.io/infineon-design-system-stencil/pr-preview-react-example/pr-1210/
Angular example: https://Infineon.github.io/infineon-design-system-stencil/pr-preview-angular-example/pr-1210/
Vanilla example: https://Infineon.github.io/infineon-design-system-stencil/pr-preview-vanilla-example/pr-1210/

tishoyanchev · 2024-05-03T11:11:01Z

It looks good. Just a new notes/issues;

is it not better if all filter-components (filter-accordion, filter-entry, filter-search, filter-type-group) are inside the same filter-folder?
There should be a cursor mouse on hover over the + icon
total items state should not be updated inside componentDidLoad inside filter-accordion
should {this.filterName} not be a label bound to the ifx-checkbox? That way, clicking over the label would focus the ifx-checkbox.
number-indicator does not update

verena-ifx · 2024-05-03T13:29:43Z

It looks good. Just a new notes/issues;

is it not better if all filter-components (filter-accordion, filter-entry, filter-search, filter-type-group) are inside the same filter-folder?

There should be a cursor mouse on hover over the + icon

total items state should not be updated inside componentDidLoad inside filter-accordion

should {this.filterName} not be a label bound to the ifx-checkbox? That way, clicking over the label would focus the ifx-checkbox.

number-indicator does not update FIXED

verena-ifx · 2024-05-03T14:09:09Z

It looks good. Just a new notes/issues;

is it not better if all filter-components (filter-accordion, filter-entry, filter-search, filter-type-group) are inside the same filter-folder?

There should be a cursor mouse on hover over the + icon

total items state should not be updated inside componentDidLoad inside filter-accordion

should {this.filterName} not be a label bound to the ifx-checkbox? That way, clicking over the label would focus the ifx-checkbox. To be honest here I followed Figma design

number-indicator does not update FIXED

verena-ifx · 2024-05-03T14:14:33Z

It looks good. Just a new notes/issues;

is it not better if all filter-components (filter-accordion, filter-entry, filter-search, filter-type-group) are inside the same filter-folder?

There should be a cursor mouse on hover over the + icon FIXED

total items state should not be updated inside componentDidLoad inside filter-accordion FIXED

should {this.filterName} not be a label bound to the ifx-checkbox? That way, clicking over the label would focus the ifx-checkbox. To be honest here I followed Figma design

number-indicator does not update FIXED

verena-ifx · 2024-05-03T14:15:32Z

It looks good. Just a new notes/issues;

is it not better if all filter-components (filter-accordion, filter-entry, filter-search, filter-type-group) are inside the same filter-folder? HMMM to be honest i prefer to have them separate, cause they are separate components after all. Plus none of them show in storybook (apart from the parent component) What do you think?

There should be a cursor mouse on hover over the + icon FIXED

total items state should not be updated inside componentDidLoad inside filter-accordion FIXED

should {this.filterName} not be a label bound to the ifx-checkbox? That way, clicking over the label would focus the ifx-checkbox. To be honest here I followed Figma design

number-indicator does not update FIXED

tishoyanchev · 2024-05-06T12:26:12Z

cursor mouse is still default when hover over the "+" icon that expands the filter.
when you type in the search, the 'delete all' icon does not appear. show-delete-icon should be set to true.
why is filter-name emitted for every component? It's a prop manually set by the user. Not sure if there's a need to emit it.
What is the purpose of having ifx-filter-entry component? It does nothing more or different than what an ifx-checkbox does. Why not use ifx-checkbox?

tishoyanchev · 2024-05-06T12:27:09Z

packages/components/src/components/filter-type-group/filter-search/filter-search.tsx

+  @Event() ifxFilterSearchChange: EventEmitter;
+
+
+  @Watch('value')


What is the purpose of this? There is no prop called "value", so this @Watch decorator is not listening to anything.

tishoyanchev · 2024-05-06T12:27:53Z

packages/components/src/components/filter-type-group/filter-entry/filter-entry.tsx

+
+  @Listen('ifxChange')
+  handleFilterEntryChange(event: CustomEvent) {
+    this.value = event.detail;


The 'value' is set twice. Once here in the @listen, and once inside the @watch.

… within (but also as standalone)

tishoyanchev

1 - cursor mouse is still default when hover over the "+" icon that expands the filter. Pic related:

2 - when you type in the search, the 'delete all' icon does not appear. show-delete-icon prop should be set to true.

3 - I am not sure if there is a point in emitting the filter-name from the ifx-filter-search, and filter-group-name from ifx-filter-accordion since both of these names are manually set by the user, and not dynamically changed.

tishoyanchev · 2024-05-08T08:19:15Z

packages/components/src/components/list/list.tsx

+  @State() showMore = false;
+  @State() selectedCount: number = 0;
+  @State() totalItems = 0;
+  @Prop() name = "";


Here, the Prop is called "list", but in the Storybook snippet, it's called "list-name".

tishoyanchev · 2024-05-08T14:11:18Z

packages/components/src/components/list/list-entry/list-entry.tsx

+      <div class="wrapper">
+        {this.type === 'checkbox' ? (
+          <div class="list-entry">
+            <ifx-checkbox size="s" value={this.value}>{this.label}</ifx-checkbox>


this.value is set twice. Once inside @watch, and once inside @listen

Setting the value in both @listen and @watch is necessary because they react to different triggers and perform different actions:

@watch('value'):
The valueChanged method is a watcher for the value prop.
Whenever the value prop changes, regardless of how the change occurred, this watcher will run.
It's responsible for reflecting the new value prop to the host element's attribute, which might be important for CSS styling or other JS that queries the host element for its value attribute.

@listen('ifxChange'):
The handleFilterEntryChange method is an event listener for the ifxChange event.
When ifxChange is emitted, presumably by the ifx-checkbox or ifx-radio-button, this method updates the value prop and emits a new event (ifxListEntryChange) to inform parent components of the change.

tishoyanchev · 2024-05-08T14:17:00Z

packages/components/src/components/filter-type-group/filter-accordion/filter-accordion.tsx

+  componentDidLoad() {
+    // Dispatch the ifxListUpdate event after the component has been fully loaded
+    const selectedItems = getInitiallySelectedItems(this.el);
+    this.count = selectedItems.length;


This Prop should not be updated inside componentDidLoad. It triggers an error warning in the console. It should be updated inside componentWillLoad() lifecycle.

I need to update it in ComponentDidLoad(), because access to the DOM is required to determine the selected items.
The update to this.count depends on the selected items that are obtained from the DOM.

I also dont get any warnings. Can you send a screenshot here as well?

You are not querying the DOM. You are only passing a reference of the component (this.el) to this function getInitiallySelectedItems().

Move these two:

const selectedItems = getInitiallySelectedItems(this.el); this.count = selectedItems.length;

inside componentWillLoad(), and it will work.

tishoyanchev · 2024-05-08T14:20:21Z

packages/components/src/components/filter-type-group/filter-accordion/filter-accordion.scss

+  flex: 1 0 0;
+  border-bottom: 1px solid tokens.$ifxColorEngineering200;
+
+  &.expanded {


The addition of this borders causes a movement. I am not sure if this should be prevented or not. If yes, then one solution is to add a transparent border.

(y) will look into this today

the borders are described like this in the figma file though. Which movement do they cause? Can you send a screenshot?

verena-ifx · 2024-05-24T10:39:26Z

s are manually set by the user, and not dynamically cha

do you mean the filterGroupName? or the label of the filter? They neeed to be emitted to be used within tihe table. Cause the table columns need to be mapped to the filters

tishoyanchev · 2024-05-24T14:16:23Z

In addition to my replies to the above comments:
1 - in storybook, the snippet is all in one line.
2 - cursor still arrow and not pointer (see 1 from the above list)
3 - when you click 'delete-all' button on the search-bar, the delete-all button remains even though the field is empty
4 - Even when the max-visible-items is higher than the existing items, the show-more button is still there. (pic for reference)

5 - when I dynamically change the max-visible-items, there's always a second delay before the changes occur.
6 - In storybook, "type=" is applied to the ifx-list and ifx-list-entry, but ifx-list does not have 'type' property.
7 - Is it really correct to apply the type to each entry-item, as opposed to the entire group? In the current way, some items can be checkbox, some can be radio at the same time. I am not sure this is in accordance with how the component should behave.

tishoyanchev · 2024-05-27T14:15:10Z

1 - remove the console.log
2 - the movement issue from above remains
3 - the warning message issue from above remains
4 - The List component snippet in Storybook is all in one line.

The other issues are fixed 👍

verena-ifx added 5 commits May 2, 2024 19:30

filter-search component created

1f711da

e2e test added

5bf8e80

removed story file, cause it is not designed to be used as a standalo…

7cdcdb7

…ne component

filter-accordion

694a055

filter-type-group component created

4891683

verena-ifx added the minor minor version bump label May 2, 2024

verena-ifx linked an issue May 2, 2024 that may be closed by this pull request

Component: Filter Type Group + sub components #1209

Open

verena-ifx self-assigned this May 2, 2024

filter-group-type component done

682b8ab

verena-ifx requested a review from tishoyanchev May 3, 2024 09:32

verena-ifx added 2 commits May 3, 2024 11:42

props added

91d822f

actions added to storybook for the filter-group-type

28ee5e0

number indicator inside filter-accordion fix

33708ab

verena-ifx added 4 commits May 3, 2024 16:15

cursor appears when hovering over the + icon of the filter-accordion

1130168

border fixed

ebc0367

restructuring and improved output for filter type group

05035bc

filter type group story updated

3e30242

tishoyanchev reviewed May 6, 2024

View reviewed changes

verena-ifx added 3 commits May 7, 2024 00:01

restructured filter component + created new list component to be used…

a57e62e

… within (but also as standalone)

restructured filter component + created new list component to be used…

cc24e77

… within (but also as standalone)

filter-type-group story bugfix

e3653bd

verena-ifx added 2 commits May 7, 2024 11:26

added list to example apps

5242482

bugfix angular deployment

14afaaa

tishoyanchev reviewed May 8, 2024

View reviewed changes

verena-ifx added 2 commits May 24, 2024 12:39

show-delete-icon set to true

4e9679a

renamed property of list component updated in filter-type-group stories

c3fe22b

Fixed remaining issues commented in the PR

bd3dd92

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhancement: Filter-type-group component created #1210

Enhancement: Filter-type-group component created #1210

verena-ifx commented May 2, 2024 •

edited by github-actions bot

github-actions bot commented May 2, 2024 •

edited

github-actions bot commented May 2, 2024

github-actions bot commented May 2, 2024

tishoyanchev commented May 3, 2024

verena-ifx commented May 3, 2024

verena-ifx commented May 3, 2024

verena-ifx commented May 3, 2024

verena-ifx commented May 3, 2024

tishoyanchev commented May 6, 2024 •

edited

tishoyanchev May 6, 2024

tishoyanchev May 6, 2024

tishoyanchev left a comment

tishoyanchev May 8, 2024

tishoyanchev May 8, 2024

verena-ifx May 24, 2024

tishoyanchev May 8, 2024

verena-ifx May 24, 2024

verena-ifx May 24, 2024

tishoyanchev May 24, 2024

tishoyanchev May 8, 2024

verena-ifx May 22, 2024

verena-ifx May 24, 2024

tishoyanchev May 24, 2024 •

edited

verena-ifx commented May 24, 2024

tishoyanchev commented May 24, 2024

tishoyanchev commented May 27, 2024

		@Event() ifxFilterSearchChange: EventEmitter;


		@Watch('value')

Enhancement: Filter-type-group component created #1210

Are you sure you want to change the base?

Enhancement: Filter-type-group component created #1210

Conversation

verena-ifx commented May 2, 2024 • edited by github-actions bot

github-actions bot commented May 2, 2024 • edited

github-actions bot commented May 2, 2024

github-actions bot commented May 2, 2024

tishoyanchev commented May 3, 2024

verena-ifx commented May 3, 2024

verena-ifx commented May 3, 2024

verena-ifx commented May 3, 2024

verena-ifx commented May 3, 2024

tishoyanchev commented May 6, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tishoyanchev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tishoyanchev May 24, 2024 • edited

Choose a reason for hiding this comment

verena-ifx commented May 24, 2024

tishoyanchev commented May 24, 2024

tishoyanchev commented May 27, 2024

verena-ifx commented May 2, 2024 •

edited by github-actions bot

github-actions bot commented May 2, 2024 •

edited

tishoyanchev commented May 6, 2024 •

edited

tishoyanchev May 24, 2024 •

edited