Extend filter fields functionality #1849
Conversation
| } | ||
| </script> | ||
|
|
||
| <style lang="scss"> |
There was a problem hiding this comment.
Since this style is not scoped you need to limit it to just this component!
| @@ -90,27 +91,6 @@ export default { | |||
|
|
|||
| methods: { | |||
| editorInit (editor) { | |||
There was a problem hiding this comment.
Move all ace-editor code to CAceEditor and make logic be tied to a prop.
This component should only parse the props passed by compose and process them so CAceEditor can use it.
| caption: v.trimStart('$'), | ||
| value: v, | ||
| })).concat(result['$']) | ||
| export const getRecordBasedSuggestions = (param = []) => { |
There was a problem hiding this comment.
Move all this logic to CInputExpressions
| const recordProperties = this.record.properties() || [] | ||
|
|
||
| const recordSuggestions = this.isRecordPage ? [ | ||
| ...(['ownerID', 'userID', 'recordID'].map(value => ({ interpolate: true, value }))), |
There was a problem hiding this comment.
userID is not tied to a record existing, it should always be available
| const { fields = [] } = this.module || {} | ||
| const moduleFields = fields.map(({ name }) => name) | ||
| const userProperties = this.$auth.user.properties() || [] | ||
| const recordValueProperties = { value: 'values', properties: Object.keys(this.record.values) || [] } |
There was a problem hiding this comment.
record can be undefined so you can move this to line 494 instead (otherwise this.record is undefined and throws an error)
| const { fields = [] } = this.module || {} | ||
| const moduleFields = fields.map(({ name }) => name) | ||
| const userProperties = this.$auth.user.properties() || [] | ||
| const recordValueProperties = { value: 'values', properties: Object.keys(this.record.values) || [] } | ||
| const recordProperties = this.record.properties() || [] |
There was a problem hiding this comment.
Same for this, you can move it to the line 493
| @@ -64,7 +64,7 @@ import { | |||
| trimChar, | |||
| } from 'corteza-webapp-compose/src/lib/record-filter.js' | |||
| import FilterToolbox from 'corteza-webapp-compose/src/components/Common/FilterToolbox.vue' | |||
| import { getRecordBasedSuggestions } from 'corteza-webapp-compose/src/lib/suggestions-tree.js' | |||
| // import { getRecordBasedSuggestions } from 'corteza-webapp-compose/src/lib/suggestions-tree.js' | |||
There was a problem hiding this comment.
guessing this isn't needed anymore
| @@ -1,61 +1,52 @@ | |||
| /* eslint-disable no-template-curly-in-string */ | |||
| export const getRecordBasedSuggestions = (param = []) => { | |||
| export const getRecordBasedSuggestions = (params) => { | |||
There was a problem hiding this comment.
Guessing this file isn't needed anymore?
| @@ -23,7 +23,7 @@ | |||
| auto-complete | |||
| lang="javascript" | |||
| :placeholder="$t('general.titlePlaceholder')" | |||
| :suggestion-tree="autoCompleteSuggestionTree" | |||
| :suggestion-params="autoCompleteParams" | |||
There was a problem hiding this comment.
lets make the naming convention like recordAutoCompleteParams
Also on this input, there is no AND/OR since this is only interpolated and not evaluated (like prefilter).
| @@ -273,7 +273,7 @@ | |||
| auto-complete | |||
| lang="javascript" | |||
| :placeholder="$t('general.visibility.condition.placeholder')" | |||
| :suggestion-tree="visibilityAutoSuggestionTree" | |||
| :suggestion-params="visibilityAutoSuggestionTree" | |||
There was a problem hiding this comment.
This one would be visibilityAutoCompleteParams
| @@ -4,8 +4,9 @@ | |||
| > | |||
There was a problem hiding this comment.
Can you rename this component to c-input-expression? Since its one expression
Also two things about the styles, the font should match the other inputs. And the input should expand if a new row is added, currently its height doesn't change.
Think about how you'll do placeholders (if possible)
|
|
||
| fontFamily: { | ||
| type: String, | ||
| default: "" |
There was a problem hiding this comment.
Single quotes, strange that te linter didnt pick it up
| ] | ||
| }, | ||
|
|
||
| processVisibilityAutoCompleteParams ({ module = this.module }) { |
There was a problem hiding this comment.
Wouldnt the methods work as functions? In my opinion mixins are a bit too much, and you can just pass the params to the function (user, record, module, isRecordPage)
| placeholder="v" | ||
| class="mb-1" | ||
| height="55.16px" |
There was a problem hiding this comment.
can the heights be in rem please. We try not to use px since the base size is defined in px and then rem is relative to that.
| @@ -106,6 +106,35 @@ export default { | |||
|
|
|||
| return !!recordPageID || !!magnifiedBlockID | |||
| }, | |||
|
|
|||
| isRecordPage () { | |||
There was a problem hiding this comment.
If this is in a mixin then you don't need it here
kelanik8
force-pushed
the
2024.3.x-extend-filter-fields-functionality
branch
from
43064e7
to
c716429
Compare
kelanik8
force-pushed
the
2024.3.x-extend-filter-fields-functionality
branch
from
c716429
to
3e1109b
Compare
The following changes are implemented
TODO: Summary
Changes in the user interface:
TODO: Add screenshots, recordings or remove this section
Checklist when submitting a final (!draft) PR