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

feat(platform): smart filter bar #7504

Merged
merged 9 commits into from
Jan 13, 2022
Merged

feat(platform): smart filter bar #7504

merged 9 commits into from
Jan 13, 2022

Conversation

N1XUS
Copy link
Contributor

@N1XUS N1XUS commented Jan 4, 2022

Related Issue(s)

closes #5291 closes #7442

Description

Created new 'Smart Filter Bar' component. -> https://deploy-preview-7504--fundamental-ngx.netlify.app/#/platform/smart-filter-bar
Refactored Platform Table data source and data provider classes.
Adjusted Platform form container component to support inline layout.
Refactored Platform table resize column service.
Adjusted Platform select component to support filling full width.
Adjusted Platform date picker to support filling full width.
Added new component to form generator - multi-input.

Screenshots

image
image
image

Please check whether the PR fulfills the following requirements

During Implementation
  1. Visual Testing:
  • visual misalignments/updates
  • check Light/Dark/HCB/HCW themes
  • RTL/LTR - proper rendering and labeling
  • responsiveness(resize)
  • Content Density (Cozy/Compact/(Condensed))
  • States - hover/disabled/focused/active/on click/selected/selected hover/press state
  • Interaction/Animation - open/close, expand/collapse, add/remove, check/uncheck
  • Mouse vs. Keyboard support
  • Text Truncation
  1. API and functional correctness
  • check for console logs (warnings, errors)
  • API boundary values
  • different combinations of components - free style
  • change the API values during testing
  1. Documentation and Example validations
  • missing API documentation or it is not understandable
  • poor examples
  • Stackblitz works for all examples
  1. Accessibility testing
  2. Browser Testing - Edge, Safari, Chrome, Firefox
PR Quality

@N1XUS N1XUS requested review from a team and removed request for valorkin and fkolar January 4, 2022 12:19
@netlify
Copy link

netlify bot commented Jan 4, 2022

✔️ Deploy Preview for fundamental-ngx ready!

🔨 Explore the source changes: c9d6068

🔍 Inspect the deploy log: https://app.netlify.com/sites/fundamental-ngx/deploys/61d43b230ffd1000074ce143

😎 Browse the preview: https://deploy-preview-7504--fundamental-ngx.netlify.app

@netlify
Copy link

netlify bot commented Jan 4, 2022

✔️ Deploy Preview for fundamental-ngx ready!

🔨 Explore the source changes: 4ccdb86

🔍 Inspect the deploy log: https://app.netlify.com/sites/fundamental-ngx/deploys/61de9c101c065000075c6957

😎 Browse the preview: https://deploy-preview-7504--fundamental-ngx.netlify.app

@SAP SAP deleted a comment from lgtm-com bot Jan 4, 2022
@N1XUS N1XUS force-pushed the feat/5291-smart-filter branch 6 times, most recently from 1e83c4e to 06d9b6e Compare January 5, 2022 09:09
@SAP SAP deleted a comment from lgtm-com bot Jan 5, 2022
Copy link
Contributor

@dmitry-stepanenko dmitry-stepanenko left a comment

Choose a reason for hiding this comment

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

Big one! Unfortunately wasn't able to play around with smart filter component due to errors while using filters

@N1XUS N1XUS force-pushed the feat/5291-smart-filter branch 6 times, most recently from 4a101f8 to ec3d6da Compare January 6, 2022 11:39
@SAP SAP deleted a comment from lgtm-com bot Jan 6, 2022
@dmitry-stepanenko
Copy link
Contributor

  1. There's no way to clear search if I want to see all results again
    image
    Also it's slightly confusing all filters are required. Maybe it would be better to have only one of them required in examples?

2.It incorrectly process absence of value

Jan-06-2022.13-30-06.mp4
  1. There's an error when trying to remove value
Jan-06-2022.13-32-31.mp4
  1. Filters count is invalid. Even if I remove one of the values, it will still increment the count
Jan-06-2022.13-35-48.mp4

@InnaAtanasova
Copy link
Contributor

Great job!
I think the Reset is not working the way it's expected:
Screen Shot 2022-01-10 at 12 24 14 PM

Is it possible to add an example with Dynamic Page or it's not in the scope of this PR?

@N1XUS
Copy link
Contributor Author

N1XUS commented Jan 10, 2022

Great job! I think the Reset is not working the way it's expected: Screen Shot 2022-01-10 at 12 24 14 PM

Is it possible to add an example with Dynamic Page or it's not in the scope of this PR?

It wasn't planned in this iteration, but I can try.
If you make changes to the filters selection in this dialog, and click on Reset button, it will reset to initial state. Since some of the fields are required and visible by default, they will be automatically selected.

@github-actions github-actions bot removed the stale label Jan 11, 2022
@N1XUS
Copy link
Contributor Author

N1XUS commented Jan 11, 2022

@InnaAtanasova example with dynamic page has been added.

Copy link
Contributor

@platon-rov platon-rov left a comment

Choose a reason for hiding this comment

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

Good, thank you!

Found that example in dynamic page doesn't scroll properly.

Screen.Recording.2022-01-11.at.15.24.55.mov

@N1XUS
Copy link
Contributor Author

N1XUS commented Jan 11, 2022

Good, thank you!

Found that example in dynamic page doesn't scroll properly.

Screen.Recording.2022-01-11.at.15.24.55.mov

It's more of the Dynamic Page issue itself. If I add more items to the table to increase the height of the dynamic page content, this behaviour won't be present

@mikerodonnell89
Copy link
Member

Styling - the additional padding provided by the form group causes the "name" input's left border to not left-align search input's left border, looks a bit strange:
Screen Shot 2022-01-11 at 9 50 35 AM

Search input's width changes when adding a character (and the X to clear the field appears) - should stay the same width

@fkolar
Copy link
Member

fkolar commented Jan 11, 2022

Do we have a design for this ? I am not even sure why we should call this smart filter bar, in the platform should be everything smart by default, to make the life of the app developer easier.

@InnaAtanasova
Copy link
Contributor

Do we have a design for this ? I am not even sure why we should call this smart filter bar, in the platform should be everything smart by default, to make the life of the app developer easier.

There's no design as it's a composition of components. In UI5 it's called Smart Filer Bar. I can send you a link to it in an email, can share it here :)

@fkolar
Copy link
Member

fkolar commented Jan 11, 2022

I believe there should be a design even for composition of the components. we have many examples in the platform like that. Cuz at the end the component is used certain why by app developer or by other component and it needs to feel right. IT does not matter what is inside. ;-(

@N1XUS N1XUS merged commit 38a9410 into main Jan 13, 2022
@N1XUS N1XUS deleted the feat/5291-smart-filter branch January 13, 2022 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(platform): select component uses label as a value SmartFilterBar Component
6 participants