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(ui): add nodes table filter + persistent UI settings #90

Merged
merged 33 commits into from
Dec 29, 2020

Conversation

ahochsteger
Copy link
Collaborator

@ahochsteger ahochsteger commented Dec 18, 2020

This is a port of the nodes table filtering feature from zwave2mqtt (see OpenZWave/Zwave2Mqtt#823).

Tasks:

  • Port nodes table filter feature
  • Port persistent UI settings
  • Refactor to generic table header templates
  • Refactor to generic filter implementation (column-independent)
  • Fix node selection bug
  • Cleanup (fix TODOs, remove console.log(), consistent naming)
  • Add filter form input validation

@ahochsteger ahochsteger changed the title feat: Nodes table filter feat: Add nodes table filter Dec 18, 2020
@coveralls
Copy link

coveralls commented Dec 18, 2020

Pull Request Test Coverage Report for Build 449658008

  • 0 of 816 (0.0%) changed or added relevant lines in 7 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-3.0%) to 25.492%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/modules/ColumnFilterHelper.test.js 0 36 0.0%
src/modules/Settings.js 0 61 0.0%
src/modules/ColumnFilterHelper.js 0 73 0.0%
src/modules/Settings.test.js 0 102 0.0%
src/components/nodes-table/nodes-table.js 0 110 0.0%
src/modules/NodeCollection.js 0 139 0.0%
src/modules/NodeCollection.test.js 0 295 0.0%
Totals Coverage Status
Change from base Build 449042411: -3.0%
Covered Lines: 1951
Relevant Lines: 7819

💛 - Coveralls

@ahochsteger
Copy link
Collaborator Author

@robertsLando this is a fully working port of the nodes table filtering - adjusted to the new columns that are now in use by zwavejs2mqtt.

There are still some issues and questions, I'd like to discuss with you to make it easier to maintain by reducing redundancy and changes cluttered over several places in the code:

  • We could make the filtering code more generic, if the headers would be consistently named after the node propertes (e.g. header product is not consistently named after the node property productLabel). Can we rename the header to productLabel?
  • Would it make sense to you to put the filtering information (from filters) directly into the headers (e.g. { text: 'ID', value: 'id', type: 'number', filter: ...})? This way the filtering could be done by just iterating the headers list and filter according to the data type. (see nodes-table.js).
  • I'd like to use a generic way for the header templates in nodes-table/index.vue as well but unfortunately I found no way to generate templates by iterating over a list. Do you have an idea, if this would be possible somehow?
  • The tests (NodeCollection.test.js) still do not seam to work, even if there has nothing changed in both this file and NodeCollection.js.

@ahochsteger ahochsteger changed the title feat: Add nodes table filter feat: Add nodes table filter + persistent UI settings Dec 19, 2020
Persisted settings:
* Dark mode
* Show hidden nodes
* Number of items per page on nodes table
* Column filters
* Column sorting
src/components/nodes-table/nodes-table.js Outdated Show resolved Hide resolved
Comment on lines 123 to 127
this.filters.manufacturer
? this.filters.manufacturer.selections
? this.filters.manufacturer.selections
: []
: []
Copy link
Member

Choose a reason for hiding this comment

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

This too

src/components/nodes-table/nodes-table.js Outdated Show resolved Hide resolved
src/components/nodes-table/nodes-table.js Outdated Show resolved Hide resolved
src/components/nodes-table/nodes-table.js Outdated Show resolved Hide resolved
src/components/nodes-table/nodes-table.js Outdated Show resolved Hide resolved
ahochsteger and others added 17 commits December 27, 2020 19:30
BREAKING CHANGE: entities names could change
Co-authored-by: Andreas Hochsteger <andreas.hochsteger@oeamtc.at>
BREAKING CHANGE: Hass entities ids will change
Co-authored-by: V Aretakis <vassilis@aretakis.eu>
* feat(ui): groups values by command class

* fix: remove genre from valueId

* fix: lint issues
* fix: undefined values

* fix: add more logging

* fix(ui): show read only list values
* add notification part 1

* add translation from states to value_template

* support both number and text

* add motion sensor icon

* add new icon for motion sensor

* add TODO for icons. Add logic on mapped default value

* patch default value, to show value_json.value when uknown mapping entry is posted

* remove extra bracket

* fix linting issues

Co-authored-by: V. Aretakis <v.aretakis@elsevier.com>
@ahochsteger ahochsteger marked this pull request as ready for review December 27, 2020 19:17
@ahochsteger
Copy link
Collaborator Author

@robertsLando it took a bit longer than I hoped but now the refactored version of the node table filter + persistent UI settings with almost all duplicate patterns is finished.

@ahochsteger ahochsteger changed the title feat: Add nodes table filter + persistent UI settings feat(ui): add nodes table filter + persistent UI settings Dec 27, 2020
Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

@ahochsteger Really nice work here! Love the actual implementation, much more cleaner! GG. There are some things that need to be fixed, give them a check when you have time, thanks!

src/components/nodes-table/ColumnFilterDate.vue Outdated Show resolved Hide resolved
src/components/nodes-table/ColumnFilterDate.vue Outdated Show resolved Hide resolved
src/components/nodes-table/ColumnFilterNumber.vue Outdated Show resolved Hide resolved
src/components/nodes-table/ColumnFilterString.vue Outdated Show resolved Hide resolved
src/modules/NodeCollection.js Outdated Show resolved Hide resolved
src/components/nodes-table/index.vue Outdated Show resolved Hide resolved
src/components/nodes-table/nodes-table.js Outdated Show resolved Hide resolved
src/components/ControlPanel.vue Show resolved Hide resolved
ahochsteger and others added 3 commits December 28, 2020 22:52
* Skip filter on invalid Regex
* Add form input validation
* Naming: until -> to
* Naming: reset filter -> reset filters
@ahochsteger
Copy link
Collaborator Author

@robertsLando thanks for your feedback!
All suggested changes and the selection bug are fixed and ready for review / approval.

Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

LGTM. GG @ahochsteger ! :)

@robertsLando
Copy link
Member

Let you sqash and merge if you are done here. I have tested your changes and everything looks good

@ahochsteger ahochsteger merged commit 91998e0 into master Dec 29, 2020
@ahochsteger ahochsteger deleted the nodes-table-filter branch December 29, 2020 10:39
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.

4 participants