feat(settings): Move "keyboard layout" from VM creation to Settings #1220

HeyRoach · 2018-08-22T03:55:44Z

No description provided.

tamazlykar · 2018-08-22T06:53:43Z

src/app/root-store/server-data/user-tags/user-tags.actions.ts

@@ -56,6 +56,10 @@ export enum UserTagsActionTypes {
  UpdateNavigationOrderSuccess = '[Resource tags API] Update "csui.user.navigation-order" tag success',
  UpdateNavigationOrderError = '[Resource tags API] Update "csui.user.navigation-order" tag error',

+  UpdateKeyboardLayoutForVms = '[Keyboard layout] Update "csui.user.vm-keyboard-layout" tag',
+  UpdateKeyboardLayoutForVmsSuccess = '[Keyboard layout] Update "csui.user.vm-keyboard-layout" tag success',
+  UpdateKeyboardLayoutForVmsError = '[Keyboard layout] Update "csui.user.vm-keyboard-layout" tag error',


Use the right sources in all your strings

tamazlykar · 2018-08-22T06:58:45Z

src/app/vm/vm-creation/vm-creation.component.html

@@ -190,15 +190,6 @@ <h4>{{ 'VM_PAGE.VM_CREATION.AFFINITY_GROUP' | translate }}</h4>
            ></cs-vm-creation-security-group-rules-manager>
          </div>

-          <div>
-            <h4>{{ 'VM_PAGE.VM_CREATION.KEYBOARD_LAYOUT' | translate }}</h4>


Remove translations if it is not used anymore

tamazlykar · 2018-08-22T07:10:47Z

src/app/root-store/server-data/user-tags/user-tags.selectors.ts

+
+export const getKeyboardLayout = createSelector(
+  getUserTagsEntities,
+  (entities): string => entities[userTagKeys.keyboardLayoutForVms] && entities[userTagKeys.keyboardLayoutForVms].value


All tags should exist see examples of other tags. Add this tag to default-config and SystemTagsService

See there is empty value if user don't have this tag

tamazlykar · 2018-08-22T07:11:50Z

src/app/reducers/vm/redux/vm-creation.effects.ts

+                  }),
+                  catchError((error) => Observable.of(new vmActions.DeploymentRequestError(error))));
+          }))
+          .catch((error) => Observable.of(new vmActions.DeploymentRequestError(error)))));


not a pipeable operator

tamazlykar · 2018-08-22T07:27:44Z

src/app/settings/containers/settings/settings.component.html

@@ -12,6 +12,10 @@
    [apiDocumentationLink]="apiDocumentationLink"
    (regenerateKeys)="onRegenerateKeys()"
  ></cs-api-settings>
+  <cs-vm-settings
+    [settings]="settings$ | async"
+    (updateKeyboardLayout)="keyboardChange($event)"


Make it looks like other bindings in the components.
It looks that its logic is different

tamazlykar · 2018-08-22T07:36:02Z

src/app/settings/settings.module.ts

 } from './components';
+import { KeyboardsComponent } from '../vm/vm-creation/keyboards/keyboards.component';


Move a component to its corresponding place

tamazlykar · 2018-08-23T03:28:12Z

src/app/vm/vm-creation/containers/vm-creation.container.ts

@@ -16,7 +16,7 @@ import { AuthService } from '../../../shared/services/auth.service';
 import { BaseTemplateModel } from '../../../template/shared';
 import { VmService } from '../../shared/vm.service';
 import { NotSelected, VmCreationState } from '../data/vm-creation-state';
-import { KeyboardLayout } from '../keyboards/keyboards.component';
+import { KeyboardLayout } from '../../../settings/components/keyboards/keyboards.component';


Do we need this type in this file and function that it uses?

@tamazlykar we use this type in a keyboard.component too.
where should we store it?

As I know you need to remove a keyboard layout from VM creation form.
This is a VM creation form and this type used only in public onKeyboardChange(keyboard: KeyboardLayout) do we still need this function and corresponding event emitter and so on?

If somwere else you need this type and it used by different modules it will be better to place type in app/shared/types

tamazlykar · 2018-08-23T03:30:23Z

src/app/vm/vm-creation/data/vm-creation-state.ts

@@ -1,6 +1,6 @@
 import { AffinityGroup, DiskOffering, InstanceGroup, ServiceOffering, SSHKeyPair, Zone } from '../../../shared/models';
 import { BaseTemplateModel } from '../../../template/shared';
-import { KeyboardLayout } from '../keyboards/keyboards.component';
+import { KeyboardLayout } from '../../../settings/components/keyboards/keyboards.component';


Do we need this type in this file and keyboard property that it uses?

tamazlykar · 2018-08-23T03:31:06Z

src/app/settings/containers/settings/settings.component.html

@@ -14,7 +14,7 @@
  ></cs-api-settings>
  <cs-vm-settings
    [settings]="settings$ | async"
-    (updateKeyboardLayout)="keyboardChange($event)"
+    (updateKeyboardLayout)="onUpdateKeyboardLayout($event)"


Make it looks like other bindings in the components.
It looks that its logic is different

# Conflicts: # src/app/core/config/default-configuration.ts # src/app/core/services/system-tags.service.ts # src/app/reducers/vm/redux/vm-creation.effects.ts # src/app/root-store/server-data/user-tags/user-tags.selectors.ts # src/app/shared/models/config/config.interface.ts

tamazlykar · 2018-09-24T02:21:15Z

src/app/reducers/vm/redux/vm-creation.effects.ts

+                    this.handleDeploymentMessages({stage: VmDeploymentStage.VM_DEPLOYED});
+
+                    return this.doCreateInstanceGroup(deployedVm, action.payload)
+                      .switchMap((virtualMachine) => this.doCopyTags(virtualMachine, action.payload));


tamazlykar · 2018-09-24T02:25:46Z

src/app/reducers/vm/redux/vm-creation.effects.ts

+                    }
+                    return new vmActions.DeploymentRequestSuccess(vmWithTags);
+                  }),
+                  catchError((error) => Observable.of(new vmActions.DeploymentRequestError(error))));


Use of instead of Observable.of because it will be deprecated in new rxjs version.
Change it in both catchError

tamazlykar · 2018-09-24T02:31:46Z

src/app/root-store/server-data/user-tags/user-tags.selectors.ts

+
+export const getKeyboardLayout = createSelector(
+  getUserTagsEntities,
+  (entities): string => entities[userTagKeys.keyboardLayoutForVms] && entities[userTagKeys.keyboardLayoutForVms].value


Let's use (entities): string => entities[userTagKeys.keyboardLayoutForVms].value without check. It can be confising why just only this selector has this check? Can it be undefined unlike the rest?

tamazlykar · 2018-09-24T02:34:25Z

src/app/settings/components/keyboards/keyboards.component.ts

@@ -0,0 +1,36 @@
+import { Component, EventEmitter, Input, Output } from '@angular/core';
+
+export enum KeyboardLayout {


Probably app/shared/types is a better place for this?

tamazlykar · 2018-09-24T02:36:41Z

src/app/settings/components/keyboards/keyboards.component.ts

+
+@Component({
+  selector: 'cs-keyboards',
+  templateUrl: 'keyboards.component.html',


Lets add changeDetection: ChangeDetectionStrategy.OnPush

tamazlykar · 2018-09-24T02:39:08Z

src/app/settings/view-models/settings.view-model.ts

@@ -7,4 +7,5 @@ export interface SettingsViewModel {
  firstDayOfWeek: DayOfWeek;
  timeFormat: TimeFormat;
  theme: string;
+  keyboardLayout: string;


Change type to KeyboardLayout

# Conflicts: # src/app/reducers/vm/redux/vm-creation.effects.ts # src/app/settings/containers/settings/settings.component.ts

feat(settings): Move "keyboard layout" from VM creation to Settings

3981416

HeyRoach assigned tamazlykar Aug 22, 2018

HeyRoach requested a review from tamazlykar August 22, 2018 03:55

tamazlykar suggested changes Aug 22, 2018

View reviewed changes

replace keyboard component, add default value for keyboard layout

96f3748

tamazlykar suggested changes Aug 23, 2018

View reviewed changes

HeyRoach added 4 commits August 23, 2018 13:14

rename methods

be125b1

replace keyboard layout enum

dc42a80

change import

dad8b78

remove unused code

bd8f41a

tamazlykar approved these changes Aug 27, 2018

View reviewed changes

HeyRoach added 4 commits September 18, 2018 10:38

update keyboard component and default configuration value

343209c

Merge branch 'master' into 861-keyboard

c092b15

fix error after refactoring

c1ae6da

tamazlykar suggested changes Sep 24, 2018

View reviewed changes

replace keyboard, small fixes

a22c40f

tamazlykar approved these changes Sep 24, 2018

View reviewed changes

HeyRoach added 2 commits September 24, 2018 13:50

Merge branch 'master' into 861-keyboard

279c747

# Conflicts: # src/app/reducers/vm/redux/vm-creation.effects.ts # src/app/settings/containers/settings/settings.component.ts

fixes after angular updating

a72764e

HeyRoach merged commit 5ff8051 into bwsw:master Sep 24, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(settings): Move "keyboard layout" from VM creation to Settings #1220

feat(settings): Move "keyboard layout" from VM creation to Settings #1220

HeyRoach commented Aug 22, 2018

tamazlykar Aug 22, 2018

tamazlykar Aug 22, 2018

tamazlykar Aug 22, 2018

tamazlykar Aug 22, 2018 •

edited

Loading

tamazlykar Aug 22, 2018

tamazlykar Aug 22, 2018

tamazlykar Aug 22, 2018

tamazlykar Aug 23, 2018

HeyRoach Aug 23, 2018

tamazlykar Aug 24, 2018

tamazlykar Aug 23, 2018

tamazlykar Aug 23, 2018

tamazlykar Sep 24, 2018

tamazlykar Sep 24, 2018

tamazlykar Sep 24, 2018

tamazlykar Sep 24, 2018

tamazlykar Sep 24, 2018

tamazlykar Sep 24, 2018

		} from './components';
		import { KeyboardsComponent } from '../vm/vm-creation/keyboards/keyboards.component';

		@@ -0,0 +1,36 @@
		import { Component, EventEmitter, Input, Output } from '@angular/core';

		export enum KeyboardLayout {

feat(settings): Move "keyboard layout" from VM creation to Settings #1220

feat(settings): Move "keyboard layout" from VM creation to Settings #1220

Conversation

HeyRoach commented Aug 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tamazlykar Aug 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tamazlykar Aug 22, 2018 •

edited

Loading