-
Notifications
You must be signed in to change notification settings - Fork 94
Display loading and save progress bars #2973
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
base: master
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -358,13 +358,25 @@ export class DeviceService { | |
| this.logService.misc('[DeviceService] load user configuration'); | ||
|
|
||
| let response: ConfigurationReply; | ||
| let progress = 0; | ||
|
|
||
| try { | ||
| await this.stopPollUhkDevice(); | ||
|
|
||
| const sendProgress = (percent: number) => { | ||
| progress = Math.max(progress, Math.min(100, percent)); | ||
| event.sender.send(IpcEvents.device.loadConfigurationProgress, progress); | ||
| }; | ||
|
|
||
| sendProgress(0); | ||
| await this.operations.waitUntilKeyboardBusy(); | ||
| const result = await this.operations.loadConfigurations(); | ||
| sendProgress(3); | ||
| const result = await this.operations.loadConfigurations((percent) => { | ||
| sendProgress(3 + Math.round(percent * 0.82)); | ||
| }); | ||
| sendProgress(88); | ||
| const modules: HardwareModules = await this.getHardwareModules(false); | ||
| sendProgress(95); | ||
|
|
||
| const hardwareConfig = getHardwareConfigFromDeviceResponse(result.hardwareConfiguration); | ||
| const uniqueId = hardwareConfig.uniqueId; | ||
|
|
@@ -385,6 +397,7 @@ export class DeviceService { | |
| info: BackupUserConfigurationInfo.Unknown | ||
| } | ||
| }; | ||
| sendProgress(100); | ||
| } catch (error) { | ||
| response = { | ||
| success: false, | ||
|
|
@@ -1298,18 +1311,32 @@ export class DeviceService { | |
|
|
||
| try { | ||
| await this.stopPollUhkDevice(); | ||
|
|
||
| let progress = 0; | ||
|
Member
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. please move this variable inside the `sendProgress. If I see well we don't use it ouside of the function |
||
| const sendProgress = (percent: number) => { | ||
| progress = Math.max(progress, Math.min(100, percent)); | ||
| event.sender.send(IpcEvents.device.saveUserConfigurationProgress, progress); | ||
| }; | ||
|
|
||
| sendProgress(0); | ||
| await backupUserConfiguration(data); | ||
| sendProgress(1); | ||
|
|
||
| this.logService.config('[DeviceService] User configuration will be saved', data.configuration); | ||
| const buffer = mapObjectToUserConfigBinaryBuffer(data.configuration); | ||
| await this.operations.saveUserConfiguration(buffer); | ||
| await this.operations.saveUserConfiguration(buffer, (percent) => { | ||
| sendProgress(1 + Math.round(percent * 0.94)); | ||
| }); | ||
|
|
||
| this._checkStatusBuffer = true; | ||
|
|
||
| if (data.saveInHistory) { | ||
| sendProgress(97); | ||
| await saveUserConfigHistoryAsync(buffer, data.deviceId, data.uniqueId); | ||
| await this.loadUserConfigFromHistory(event); | ||
| } | ||
|
|
||
| sendProgress(100); | ||
| response.success = true; | ||
| } catch (error) { | ||
| this.logService.error('[DeviceService] Transferring error', error); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -308,11 +308,36 @@ export class UhkOperations { | |
| * Return with the actual UserConfiguration from UHK Device | ||
| * @returns {Promise<Buffer>} | ||
| */ | ||
| public async loadConfigurations(): Promise<LoadConfigurationsResult> { | ||
| public async loadConfigurations(onProgress?: (percent: number) => void): Promise<LoadConfigurationsResult> { | ||
| try { | ||
| await this.waitUntilKeyboardBusy(); | ||
| const userConfiguration = await this.loadConfiguration(ConfigBufferId.validatedUserConfig); | ||
| const hardwareConfiguration = await this.loadConfiguration(ConfigBufferId.hardwareConfig); | ||
| onProgress?.(0); | ||
|
|
||
| const configSizes = await this.getConfigSizesFromKeyboard(); | ||
| let userConfigSize = configSizes.userConfig; | ||
| const hardwareConfigSize = configSizes.hardwareConfig; | ||
|
|
||
| const reportTransferProgress = (userOffset: number, hardwareOffset: number) => { | ||
| const totalBytes = Math.max(userConfigSize + hardwareConfigSize, 1); | ||
| const transferredBytes = userOffset + hardwareOffset; | ||
|
|
||
| onProgress?.(Math.round(Math.min(transferredBytes / totalBytes, 1) * 100)); | ||
| }; | ||
|
|
||
| const userConfiguration = await this.loadConfiguration( | ||
| ConfigBufferId.validatedUserConfig, | ||
| (offset, configSize) => { | ||
| userConfigSize = configSize; | ||
| reportTransferProgress(offset, 0); | ||
| } | ||
| ); | ||
| const hardwareConfiguration = await this.loadConfiguration( | ||
| ConfigBufferId.hardwareConfig, | ||
| (offset) => { | ||
| reportTransferProgress(userConfigSize, offset); | ||
| } | ||
| ); | ||
| reportTransferProgress(userConfigSize, hardwareConfigSize); | ||
|
Member
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. Do we need this last progress reporting? |
||
|
|
||
| return { | ||
| userConfiguration: JSON.stringify(convertBufferToIntArray(userConfiguration)), | ||
|
|
@@ -327,7 +352,10 @@ export class UhkOperations { | |
| * Return with the actual user / hardware fonfiguration from UHK Device | ||
| * @returns {Promise<Buffer>} | ||
| */ | ||
| public async loadConfiguration(configBufferId: ConfigBufferId): Promise<Buffer> { | ||
| public async loadConfiguration( | ||
| configBufferId: ConfigBufferId, | ||
| onProgress?: (offset: number, configSize: number) => void | ||
| ): Promise<Buffer> { | ||
| const configBufferIdToName = ['HardwareConfig', 'StagingUserConfig', 'ValidatedUserConfig']; | ||
| const configName = configBufferIdToName[configBufferId]; | ||
|
|
||
|
|
@@ -360,6 +388,8 @@ export class UhkOperations { | |
| configSize = originalConfigSize; | ||
| } | ||
| } | ||
|
|
||
| onProgress?.(offset, configSize); | ||
| } | ||
|
|
||
| return configBuffer; | ||
|
|
@@ -393,8 +423,15 @@ export class UhkOperations { | |
| }; | ||
| } | ||
|
|
||
| public async saveUserConfiguration(buffer: Buffer): Promise<void> { | ||
| public async saveUserConfiguration(buffer: Buffer, onProgress?: (percent: number) => void): Promise<void> { | ||
| let lastProgress = 0; | ||
|
Member
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. please move this variable initialisation inside the |
||
| const reportProgress = (percent: number) => { | ||
| lastProgress = Math.max(lastProgress, Math.min(100, Math.round(percent))); | ||
| onProgress?.(lastProgress); | ||
| }; | ||
|
|
||
| try { | ||
| reportProgress(0); | ||
| this.logService.usbOps('[DeviceOperation] USB[T]: Write user configuration to keyboard'); | ||
| let shouldRecalculateLength = false; | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
|
|
@@ -423,11 +460,20 @@ export class UhkOperations { | |
|
|
||
| const resultBuffer = new UhkBuffer(UHK_EEPROM_SIZE) | ||
| userConfiguration.toBinary(resultBuffer) | ||
| await this.sendConfigToKeyboard(resultBuffer.getBufferContent(), true); | ||
| const configBuffer = resultBuffer.getBufferContent(); | ||
|
|
||
| reportProgress(2); | ||
|
Member
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 worth to the |
||
| await this.sendConfigToKeyboard(configBuffer, true, (bytesSent, totalBytes) => { | ||
| reportProgress(2 + bytesSent / totalBytes * 83); | ||
| }); | ||
| reportProgress(86); | ||
| await this.applyConfiguration(); | ||
| reportProgress(90); | ||
| this.logService.usbOps('[DeviceOperation] USB[T]: Write user configuration to EEPROM'); | ||
| await this.writeConfigToEeprom(ConfigBufferId.validatedUserConfig); | ||
| reportProgress(94); | ||
| await this.waitUntilKeyboardBusy(); | ||
| reportProgress(96); | ||
| } catch (error) { | ||
| this.logService.error('[DeviceOperation] Transferring error', error); | ||
| throw error; | ||
|
|
@@ -916,14 +962,23 @@ export class UhkOperations { | |
| * @returns {Promise<void>} | ||
| * @private | ||
| */ | ||
| private async sendConfigToKeyboard(buffer: Buffer, isUserConfiguration): Promise<void> { | ||
| private async sendConfigToKeyboard( | ||
| buffer: Buffer, | ||
| isUserConfiguration, | ||
| onProgress?: (bytesSent: number, totalBytes: number) => void | ||
| ): Promise<void> { | ||
| const command = isUserConfiguration | ||
| ? UsbCommand.WriteStagingUserConfig | ||
| : UsbCommand.WriteHardwareConfig; | ||
|
|
||
| const fragments = getTransferBuffers(command, buffer); | ||
| for (const fragment of fragments) { | ||
| await this.device.write(fragment); | ||
| for (let i = 0; i < fragments.length; i++) { | ||
| await this.device.write(fragments[i]); | ||
| const bytesSent = Math.min( | ||
| buffer.length, | ||
| Math.round((i + 1) / fragments.length * buffer.length) | ||
| ); | ||
| onProgress?.(bytesSent, buffer.length); | ||
|
Member
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 we could follow the other progress pattern when the callback called only with the |
||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,15 @@ | ||
| <button class="btn btn-primary" | ||
| <button class="btn btn-primary progress-button" | ||
| (click)="onClicked()" | ||
| [class.progress-button--active]="state.showProgress" | ||
| [disabled]="state.showProgress"> | ||
| <fa-icon *ngIf="state.showProgress" | ||
| [icon]="faSpinner" | ||
| animation="spin"></fa-icon> | ||
| {{state.text}} | ||
| <span class="progress-button__fill" | ||
| role="progressbar" | ||
| [attr.aria-valuenow]="state.progressPercent ?? 0" | ||
|
Member
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. please set the default value in the ngrx state and don't use the nullish coalescing operator (??). |
||
| aria-valuemin="0" | ||
| aria-valuemax="100" | ||
| [style.width.%]="state.showProgress ? (state.progressPercent ?? 0) : 0"> | ||
| </span> | ||
| <span class="progress-button__content"> | ||
| {{state.text}} | ||
| </span> | ||
| </button> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,23 @@ | ||
| button { | ||
| min-width: 150px; | ||
| } | ||
|
|
||
| .progress-button { | ||
| position: relative; | ||
| overflow: hidden; | ||
| } | ||
|
|
||
| .progress-button__fill { | ||
| position: absolute; | ||
| top: 0; | ||
| left: 0; | ||
| bottom: 0; | ||
| background-color: var(--color-loading-progress-bar-fill); | ||
| opacity: 0.35; | ||
| transition: width 200ms ease-out; | ||
| } | ||
|
|
||
| .progress-button__content { | ||
| position: relative; | ||
| z-index: 1; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,22 @@ | ||
| <div class="uhk-message-wrapper"> | ||
| <uhk-agent-icon *ngIf="showLogo" | ||
| class="agent-logo" | ||
| [ngClass]="{'spin-logo': rotateLogo}"> | ||
| </uhk-agent-icon> | ||
| <div class="messages"> | ||
| <h1> {{ header }} </h1> | ||
| <h2 *ngIf="!smallText"> {{ subtitle }} </h2> | ||
| <div *ngIf="smallText"> {{ subtitle }} </div> | ||
| <p *ngIf="description"> {{ description }} </p> | ||
| <div class="uhk-message-content" [class.with-progress-bar]="showProgressBar"> | ||
| <div class="uhk-message-main"> | ||
| <uhk-agent-icon *ngIf="showLogo" | ||
| class="agent-logo" | ||
| [ngClass]="{'spin-logo': rotateLogo}"> | ||
| </uhk-agent-icon> | ||
| <div class="messages"> | ||
| <h1> {{ header }} </h1> | ||
| <h2 *ngIf="!smallText"> {{ subtitle }} </h2> | ||
| <div *ngIf="smallText"> {{ subtitle }} </div> | ||
| <p *ngIf="description"> {{ description }} </p> | ||
| </div> | ||
| </div> | ||
| <div *ngIf="showProgressBar" class="loading-progress-bar-wrapper"> | ||
| <progress class="loading-progress-bar" | ||
| [value]="progressPercent" | ||
| max="100"> | ||
| </progress> | ||
| </div> | ||
| </div> | ||
| </div> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,12 +5,63 @@ | |
| justify-content: center; | ||
| } | ||
|
|
||
| .uhk-message-content { | ||
| display: flex; | ||
| flex-direction: column; | ||
| align-items: stretch; | ||
| } | ||
|
|
||
| .uhk-message-main { | ||
| display: flex; | ||
| align-items: center; | ||
| } | ||
|
|
||
| .agent-logo { | ||
| margin: 1.25em; | ||
| height: 8em; | ||
| width: 8em; | ||
| } | ||
|
|
||
| .with-progress-bar .agent-logo { | ||
|
Member
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. Do we need this special behavior? Does not enough modify the |
||
| margin-bottom: 1.5em; | ||
| } | ||
|
|
||
| .loading-progress-bar-wrapper { | ||
| width: 100%; | ||
| padding: 0 1.25em; | ||
| } | ||
|
|
||
| .loading-progress-bar { | ||
| display: block; | ||
| width: 100%; | ||
| height: 1rem; | ||
| appearance: none; | ||
| border: none; | ||
| border-radius: 0.375rem; | ||
| overflow: hidden; | ||
| background-color: var(--color-loading-progress-bar-bg); | ||
|
|
||
| &::-webkit-progress-inner-element { | ||
| border: none; | ||
| } | ||
|
|
||
| &::-webkit-progress-bar { | ||
| background-color: var(--color-loading-progress-bar-bg); | ||
| } | ||
|
|
||
| &::-webkit-progress-value { | ||
| background-color: var(--color-loading-progress-bar-fill); | ||
| border-radius: 0; | ||
| transition: inline-size 200ms ease-out; | ||
| } | ||
|
|
||
| &::-moz-progress-bar { | ||
| background-color: var(--color-loading-progress-bar-fill); | ||
| border-radius: 0; | ||
| transition: inline-size 200ms ease-out; | ||
| } | ||
| } | ||
|
|
||
| .message { | ||
| display: flex; | ||
| flex-direction: column; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,8 @@ | ||
| import { Component } from '@angular/core'; | ||
| import { Store } from '@ngrx/store'; | ||
| import { Observable } from 'rxjs'; | ||
|
|
||
| import { AppState, getConfigurationLoadingProgress } from '../../store'; | ||
|
|
||
| @Component({ | ||
| selector: 'loading-device', | ||
|
|
@@ -7,11 +11,15 @@ import { Component } from '@angular/core'; | |
| <uhk-message header="Loading keyboard configuration..." | ||
| subtitle="Hang tight!" | ||
| [showLogo]="true" | ||
| [rotateLogo]="true"></uhk-message> | ||
| [rotateLogo]="true" | ||
| [showProgressBar]="true" | ||
| [progressPercent]="(progressPercent$ | async) ?? 0"></uhk-message> | ||
|
Member
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. please set the default value at state or selector level. Try to avoid from ?? operator |
||
| `, | ||
| }) | ||
| export class LoadingDevicePageComponent { | ||
| progressPercent$: Observable<number>; | ||
|
|
||
| constructor() { | ||
| constructor(store: Store<AppState>) { | ||
| this.progressPercent$ = store.select(getConfigurationLoadingProgress); | ||
| } | ||
| } | ||
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.
Please move this variable initialisation inside the
sendProgressfunction. If I see weel we don't use it outside