Convert AccountEdit and AccountEditAvatar to typescript #11209
Conversation
…s request was saved but cancelled)
Because when navigating away during an update, the actually settings may or may not have been successfully updated if the page's state is restored, so the 'default' value may actually be wrong without refreshing the page.
| const elem = this.main[0]; | ||
|
|
||
| return elem instanceof HTMLElement ? elem : null; |
There was a problem hiding this comment.
might as well just use querySelector and assign on page load or something
| $(document).on('dragover', '.js-account-edit-avatar', this.overlayHover); | ||
| } | ||
|
|
||
| initialize = () => { |
resources/js/core/account-edit.ts
Outdated
| private readonly handleInputChange = (e: ContainerEvent) => { | ||
| const container = e.currentTarget; | ||
|
|
||
| if (container.dataset.accountEditAutoSubmit !== '1') { |
There was a problem hiding this comment.
might as well use different class altogether. Saves checking dataset everywhere
resources/js/core/account-edit.ts
Outdated
| container.state ??= new AccountEditState(container, this.core); | ||
| container.state.saving(); |
There was a problem hiding this comment.
create a this.getState(container)?
| private getValue() { | ||
| let value: string | string[] | undefined; | ||
|
|
||
| if (this.dataset.accountEditType === 'array') { |
| this.timeout = window.setTimeout(() => { | ||
| this.dataset.accountEditState = ''; |
There was a problem hiding this comment.
setTimeout(this.clear, ...)?
| this.dataset.accountEditState = 'saving'; | ||
| } | ||
|
|
||
| private getMultiValue() { |
There was a problem hiding this comment.
this and getValue can probably be merged into data?
(and if anything this is more like getMultiData since it includes the field name)
| let value: string | string[] | undefined; | ||
|
|
||
| if (this.dataset.accountEditType === 'array') { | ||
| value = []; |
There was a problem hiding this comment.
the initial empty string is required to allow unselecting everything
| if (value == null) { | ||
| throw new Error('missing radio value'); | ||
| } |
There was a problem hiding this comment.
shouldn't this be in the radio case? 🤨 or at least the message implies so
There was a problem hiding this comment.
huh, I guess it does narrow the type down properly for the return 🤔
Moved the extensions on the html element handling the state into a separate class.
Also removed the
value == prevValuecheck since the request can be aborted before or after the server has already processed it, which means the last value may or may not be wrong already.A proper check would be to check against the known saved value or initial value which doesn't happen since the way it's currently set up, the first known value is after the change.only fixed initial value check for where thedefaultValueis usable (so not checkboxes)Removed the no update on default check because things may not update as expected when navigating away and restoring the page state, regardless of cancelling the xhr or not due to the actually state being nebulous unless all the settings are fully reloaded.