-
Notifications
You must be signed in to change notification settings - Fork 65
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
Table: Datepicker, AutoComplete, Select Cell Editor #2387
Changes from 37 commits
b95a4c2
725b446
f6b56dc
f537d06
be4d8fd
3deb650
b93b6b5
fb830ed
a56c59e
b8ecefd
60c0326
c6f0351
f5ae27c
dd0cc92
0b56a8e
b71aa9e
38a1ce2
b9af3e8
d14a253
dc87b14
f0a667f
e6daf3c
79bb111
caa9de6
b0d094a
72aff7a
a41f396
70908d1
fc648ee
1a82947
a7fe216
ec59779
df7ffc8
3e81f63
a3dbb68
9328a6b
8e27466
9bc859c
317ece5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -86,6 +86,10 @@ | |
export class ModusTable { | ||
@Element() element: HTMLElement; | ||
|
||
@Prop() editableCellId: string[] = []; | ||
@Watch('editableCellId') onEditableCellIdsChange(newVal: string[]) { | ||
console.log('editableCellIds', newVal); | ||
} | ||
/** (Required) To display headers in the table. */ | ||
@Prop({ mutable: true }) columns!: ModusTableColumn<unknown>[]; | ||
@Watch('columns') onColumnsChange(newVal: ModusTableColumn<unknown>[]) { | ||
|
@@ -373,6 +377,7 @@ | |
this.setTableState(initialTableState); | ||
this.onRowsExpandableChange(this.rowsExpandable); | ||
this.initializeTable(); | ||
console.log('editableCellId232s', this.editableCellId); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed |
||
} | ||
|
||
componentWillRender(): void { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,16 +4,17 @@ | |
KEYBOARD_ENTER, | ||
CELL_EDIT_TYPE_AUTOCOMPLETE, | ||
CELL_EDIT_TYPE_DATE, | ||
CELL_EDIT_TYPE_DROPDOWN, | ||
CELL_EDIT_TYPE_SELECT, | ||
CELL_EDIT_TYPE_TEXT, | ||
CELL_EDIT_TYPE_INT, | ||
KEYBOARD_UP, | ||
KEYBOARD_DOWN, | ||
} from '../../../modus-table.constants'; | ||
|
||
import { | ||
ModusTableCellAutocompleteEditorArgs, | ||
ModusTableCellDateEditorArgs, | ||
ModusTableCellDropdownEditorArgs, | ||
ModusTableCellSelectEditorArgs, | ||
ModusTableCellEditorArgs, | ||
} from '../../../models/modus-table.models'; | ||
import { ModusDateInputEventDetails } from '../../../../modus-date-input/utils/modus-date-input.models'; | ||
|
@@ -25,28 +26,40 @@ | |
}) | ||
export class ModusTableCellEditor { | ||
@Prop() args: ModusTableCellEditorArgs; | ||
@Prop() value: string; | ||
@Prop() dataType: string; | ||
@Prop() value: any; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replaced with unknown |
||
@Prop() type: string; | ||
@Prop() valueChange: (newValue: string) => void; | ||
@Prop() keyDown: (e: KeyboardEvent, newValue: string) => void; | ||
|
||
private editedValue: string; | ||
private editedValue: any; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replaced with unknown |
||
private inputElement: HTMLElement; | ||
private outsideClickListener: (event: any) => void; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't this be of type There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, changed |
||
|
||
connectedCallback(): void { | ||
this.editedValue = this.value; | ||
} | ||
|
||
componentDidLoad(): void { | ||
if (this.inputElement['focusInput']) this.inputElement['focusInput'](); | ||
|
||
this.outsideClickListener = (event) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it may be cleaner to do @Listen('click', { target: 'document' })
handleDocumentClick(event: MouseEvent) {
if (this.type != 'date'){
return;
}
const target = event.target as HTMLElement;
if (!this.inputElement.contains(event.target)) {
this.handleBlur();
}
} Also I am a firm believer that we should always use if (this.type == 'date') document.addEventListener('click', this.outsideClickListener); is less readable and potentially more error prone than if (this.type == 'date') {
document.addEventListener('click', this.outsideClickListener);
} If we were to keep the original logic, there is no reason to define There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replaced with the above logic |
||
if (!this.inputElement.contains(event.target)) { | ||
this.handleBlur(); | ||
} | ||
}; | ||
if (this.type == 'date') document.addEventListener('click', this.outsideClickListener); | ||
} | ||
disconnectedCallback(): void { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ultimately if we use the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replaced the callback logic with listen |
||
document.removeEventListener('click', this.outsideClickListener); | ||
} | ||
|
||
handleBlur: () => void = () => { | ||
this.valueChange(this.editedValue); | ||
this.valueChange(this.editedValue as string); | ||
}; | ||
|
||
handleKeyDown: (e: KeyboardEvent) => void = (e) => { | ||
this.keyDown(e, this.editedValue); | ||
this.keyDown(e, this.editedValue as string); | ||
}; | ||
|
||
getDefaultProps = (ariaLabel) => ({ | ||
|
@@ -88,10 +101,12 @@ | |
); | ||
} | ||
|
||
renderDropdownInput(): JSX.Element[] { | ||
renderSelectInput(): JSX.Element[] { | ||
const valueKey = 'display'; | ||
const options = (this.args as ModusTableCellDropdownEditorArgs)?.options || []; | ||
const selectedOption = options.find((option) => option[valueKey] === this.value); | ||
const args = this.args as ModusTableCellSelectEditorArgs; | ||
const options = args?.options || []; | ||
const optionsDisplayProp = args?.optionsDisplayProp || valueKey; | ||
const selectedOption = options.find((option) => option[optionsDisplayProp] === this.value) as any; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Type would be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replaced with unknown |
||
|
||
function handleEnter(e: KeyboardEvent, callback: (e: KeyboardEvent) => void) { | ||
const code = e.key.toLowerCase(); | ||
|
@@ -105,13 +120,20 @@ | |
<modus-select | ||
{...this.getDefaultProps('Select input')} | ||
value={selectedOption} | ||
options-display-prop="display" | ||
options-display-prop={optionsDisplayProp} | ||
size="large" | ||
options={options} | ||
onInputBlur={this.handleBlur} | ||
onKeyDown={(e) => handleEnter(e, this.handleKeyDown)} | ||
onValueChange={(e: CustomEvent<unknown>) => { | ||
this.editedValue = e.detail[valueKey]; | ||
if (this.dataType === 'badge') { | ||
const { display, ...restProps } = e.detail as any; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replaced with unknown |
||
this.editedValue = { ...restProps, text: display }; | ||
} else if (this.dataType === 'link') { | ||
this.editedValue = e.detail; | ||
} else { | ||
this.editedValue = e.detail[valueKey]; | ||
} | ||
}}></modus-select> | ||
</div> | ||
); | ||
|
@@ -121,45 +143,72 @@ | |
const valueKey = 'value'; | ||
const format = (this.args as ModusTableCellDateEditorArgs)?.format; | ||
return ( | ||
<modus-date-picker> | ||
<modus-date-picker onBlur={this.handleBlur} onClick={(e: MouseEvent) => e.stopPropagation()}> | ||
<modus-date-input | ||
{...this.getDefaultProps('Date input')} | ||
format={format} | ||
size="large" | ||
show-calendar-icon="true" | ||
value={this.value} | ||
onClick={(e: MouseEvent) => e.stopPropagation()} | ||
onValueChange={(e: CustomEvent<ModusDateInputEventDetails>) => | ||
(this.editedValue = e.detail[valueKey]) | ||
}></modus-date-input> | ||
onValueChange={(e: CustomEvent<ModusDateInputEventDetails>) => { | ||
this.editedValue = e.detail[valueKey]; | ||
}}></modus-date-input> | ||
</modus-date-picker> | ||
); | ||
} | ||
|
||
renderAutocompleteInput(): JSX.Element[] { | ||
const options = ((this.args as ModusTableCellDropdownEditorArgs)?.options || []) as ModusAutocompleteOption[] | string[]; | ||
let options, selectedOption; | ||
const args = this.args as ModusTableCellAutocompleteEditorArgs; | ||
// const { noResultsFoundText, noResultsFoundSubtext, showNoResultsFoundMessage, showOptionsOnFocus } = args; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove commented out code There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed |
||
if (this.dataType === 'badge') { | ||
options = args?.options.map((option: any) => option.text) as ModusAutocompleteOption[] | string[]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replaced with unknown |
||
selectedOption = this.value['text']; | ||
} else if (this.dataType === 'link') { | ||
options = args?.options.map((option: any) => option.display) as ModusAutocompleteOption[] | string[]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replaced with unknown |
||
selectedOption = this.value['display']; | ||
} else { | ||
options = (args?.options || []) as ModusAutocompleteOption[] | string[]; | ||
selectedOption = this.editedValue; | ||
} | ||
return ( | ||
<modus-autocomplete | ||
{...this.getDefaultProps('Autocomplete input')} | ||
include-search-icon="false" | ||
size="large" | ||
// onClick={(e: MouseEvent) => e.stopPropagation()} | ||
options={options} | ||
// onInputBlur={this.handleBlur} | ||
onOptionSelected={(e: CustomEvent<string>) => { | ||
this.editedValue = e.detail; | ||
this.valueChange(this.editedValue); | ||
}} | ||
// value={selectedOption} | ||
// onKeyDown={(e) => e.stopPropagation()} | ||
></modus-autocomplete> | ||
<div class="autocomplete-container"> | ||
<modus-autocomplete | ||
{...this.getDefaultProps('Autocomplete input')} | ||
include-search-icon="false" | ||
size="medium" | ||
//onClick={(e: MouseEvent) => e.stopPropagation()} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove commented out code There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed |
||
options={options} | ||
onBlur={this.handleBlur} | ||
onKeyDown={(e) => e.stopPropagation()} | ||
onOptionSelected={(e: CustomEvent<string>) => { | ||
if (this.dataType === 'badge') { | ||
args?.options.map((option: any) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replaced with unknown |
||
if (option.text == e.detail) { | ||
this.editedValue = option; | ||
} | ||
}); | ||
} else if (this.dataType === 'link') { | ||
args?.options.map((option: any) => { | ||
if (option.display == e.detail) { | ||
this.editedValue = option; | ||
} | ||
}); | ||
} else { | ||
this.editedValue = e.detail; | ||
} | ||
}} | ||
value={selectedOption} | ||
// onKeyDown={(e) => e.stopPropagation()} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove any commented out code There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed |
||
></modus-autocomplete> | ||
</div> | ||
); | ||
} | ||
|
||
renderEditor(): JSX.Element[] { | ||
switch (this.type) { | ||
case CELL_EDIT_TYPE_DROPDOWN: | ||
return this.renderDropdownInput(); | ||
case CELL_EDIT_TYPE_SELECT: | ||
return this.renderSelectInput(); | ||
case CELL_EDIT_TYPE_AUTOCOMPLETE: | ||
return this.renderAutocompleteInput(); | ||
case CELL_EDIT_TYPE_DATE: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
console.log
, and was thiseditableCellId
prop only used for debugging? If so, remove, otherwise we will need to discuss its purpose.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops committed it by mistake, will remove it