Refactors Labels Component to Shared TKG Labels #2515

mpanchajanya · 2022-05-28T03:07:50Z

What this PR does / why we need it

Which issue(s) this PR fixes

Fixes #

Describe testing done for PR

Release note

1) new FormUtils.addDynamicControl method
2) usage of FormArray for labels
3) Usage of Reactive Form Validations from Rxweb
4) tkg-labels shared component

PR Checklist

Squash the commits into one or a small number of logical commits
Use good commit messages
Ensure PR contains terms all contributors can understand and links all contributors can access

Additional information

Special notes for your reviewer

github-actions · 2022-05-28T03:08:43Z

Hi @mpanchajanya! And thank you for opening your first Pull Request. Someone will review it soon. Thank you for committing to making Tanzu Framework better.

miclettej · 2022-05-31T16:00:06Z

pkg/v1/tkg/web/src/app/app.module.ts

-import { BrowserModule } from '@angular/platform-browser';
-import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
-
+import {NgModule} from '@angular/core';


please update your IDE settings to revert removal of the spaces between {} and []

swalner-vmware · 2022-05-31T16:00:45Z

.gitignore

I think we don't generally add IDE-specific files to the repo (you can add them locally).
IFF we do decide to do so, I would suggest just ignoring the .idea directory, rather than all these specific files.

moved .idea files to my local global .gitignore

swalner-vmware · 2022-05-31T16:02:24Z

pkg/v1/tkg/web/src/app/app.module.ts

-import { BrowserModule } from '@angular/platform-browser';
-import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
-
+import {NgModule} from '@angular/core';


nit: we're trying to keep imports alphabetized (within the three sections of Angular, 3rd party, app)

.../src/app/views/landing/wizard/shared/components/widgets/tkg-labels/tkg-labels.component.html

swalner-vmware · 2022-05-31T16:13:56Z

...rc/app/views/landing/wizard/shared/components/steps/metadata-step/metadata-step.component.ts

    }

    protected storeUserData() {
        this.storeUserDataFromMapping(this.supplyStepMapping());
        this.storeDefaultDisplayOrder(this.supplyStepMapping());
    }
+
+    private supplyStepMapping(): StepMapping {
+        return this.stepMapping ?? MetadataStepMapping;


I'm not sure it makes sense to return the default MetadataStepMapping if this.stepMapping is null/undefined. Is this.stepMapping === null/undefined a legitimate state?

swalner-vmware · 2022-05-31T16:15:02Z

...views/landing/wizard/shared/components/widgets/tkg-labels/interfaces/tkg-labels.interface.ts

+    };
+}
+
+export interface TKGLabel<K = string, V = string> {


Is there a case where we would use TKGLabel for a type other than string?

refactored to use string

swalner-vmware · 2022-05-31T16:15:18Z

.../src/app/views/landing/wizard/shared/components/widgets/tkg-labels/tkg-labels.component.html

+                <clr-tooltip>
+                    <clr-icon a11yTooltipTrigger aria-live="assertive" shape="info-circle" size="22"></clr-icon>
+                    <clr-tooltip-content *clrIfOpen clrPosition="top-right" clrSize="lg">
+						<span>


nit: odd indentation here

fixed the indentation

swalner-vmware · 2022-05-31T16:19:12Z

...eb/src/app/views/landing/wizard/shared/components/widgets/tkg-labels/tkg-labels.component.ts

+            this.labelsFormArray.valueChanges
+                .pipe(takeUntil(this.stopSubscriptions$), distinctUntilChanged())
+                .subscribe(() => {
+                    setTimeout(() => {


Curious about this setTimeout(): is it necessary, and if so, do we know why?

Was using it for testing. removed now

miclettej · 2022-05-31T16:51:49Z

.gitignore

@@ -47,4 +47,11 @@ hack/providers-sync-tools/**/build
 # Build artifacts related to packages


probably best to just revert any changes to this file. I'm not sure why it got updated.

moved .idea files to my local global .gitignore

...app/views/landing/wizard/shared/components/steps/metadata-step/metadata-step.fieldmapping.ts

pkg/v1/tkg/web/src/app/app.module.ts

vuil · 2022-06-03T06:22:52Z

welcome @mpanchajanya !
nit: please provide a more describe PR title

# Conflicts: # pkg/v1/tkg/web/package-lock.json

github-actions · 2022-06-03T18:35:57Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/2515/20220603183555/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

codecov · 2022-06-03T19:24:05Z

Codecov Report

Merging #2515 (9f324f9) into main (4494221) will increase coverage by 0.07%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2515      +/-   ##
==========================================
+ Coverage   42.32%   42.39%   +0.07%     
==========================================
  Files         398      398              
  Lines       39693    39704      +11     
==========================================
+ Hits        16799    16834      +35     
+ Misses      21308    21287      -21     
+ Partials     1586     1583       -3

Impacted Files	Coverage Δ
pkg/v1/tkg/tkgpackageclient/package_update.go	`83.57% <0.00%> (-1.43%)`	⬇️
addons/controllers/clusterbootstrap_controller.go	`69.20% <0.00%> (+0.98%)`	⬆️
...ons/controllers/packageinstallstatus_controller.go	`82.79% <0.00%> (+2.15%)`	⬆️
...kr/webhook/cluster/tkr-resolver/cluster/cluster.go	`69.60% <0.00%> (+6.91%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4494221...9f324f9. Read the comment docs.

miclettej

Very nice!

github-actions · 2022-06-06T16:00:03Z

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/2515/20220606160000/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

…-tanzu#2515) * Issue vmware-tanzu#1620 - TKG Labels * update lock * add spaces on imports

vmwclabot added the cla-not-required label May 28, 2022

miclettej reviewed May 31, 2022

View reviewed changes

swalner-vmware reviewed May 31, 2022

View reviewed changes

miclettej reviewed May 31, 2022

View reviewed changes

.../src/app/views/landing/wizard/shared/components/widgets/tkg-labels/tkg-labels.component.html Outdated Show resolved Hide resolved

swalner-vmware reviewed May 31, 2022

View reviewed changes

miclettej reviewed May 31, 2022

View reviewed changes

...app/views/landing/wizard/shared/components/steps/metadata-step/metadata-step.fieldmapping.ts Show resolved Hide resolved

raymondz1 reviewed May 31, 2022

View reviewed changes

pkg/v1/tkg/web/src/app/app.module.ts Outdated Show resolved Hide resolved

Issue #1620 - TKG Labels

6985de7

mpanchajanya marked this pull request as ready for review June 2, 2022 19:56

mpanchajanya requested review from a team as code owners June 2, 2022 19:56

mpanchajanya changed the title ~~TKG Labels~~ Refactors Labels Component to Shared TKG Labels Jun 3, 2022

mpanchajanya requested review from miclettej, raymondz1 and swalner-vmware June 3, 2022 14:30

mpanchajanya added 2 commits June 3, 2022 12:08

Merge branch 'main' into pj/tkg-labels

d3a30b8

# Conflicts: # pkg/v1/tkg/web/package-lock.json

update lock

00ca420

add spaces on imports

9f324f9

miclettej approved these changes Jun 6, 2022

View reviewed changes

miclettej added ok-to-merge PRs should be labelled with this before merging area/ui Specific UI issue/enhancement labels Jun 6, 2022

miclettej merged commit e723ae6 into vmware-tanzu:main Jun 6, 2022

ankeesler pushed a commit to ankeesler/tanzu-framework that referenced this pull request Jun 14, 2022

Kickstart UI: Refactors Labels Component to Shared TKG Labels (vmware…

bd86b53

…-tanzu#2515) * Issue vmware-tanzu#1620 - TKG Labels * update lock * add spaces on imports

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactors Labels Component to Shared TKG Labels #2515

Refactors Labels Component to Shared TKG Labels #2515

mpanchajanya commented May 28, 2022 •

edited

Loading

github-actions bot commented May 28, 2022

miclettej May 31, 2022

mpanchajanya Jun 1, 2022

swalner-vmware May 31, 2022

mpanchajanya Jun 1, 2022

swalner-vmware May 31, 2022

mpanchajanya Jun 1, 2022

swalner-vmware May 31, 2022

mpanchajanya Jun 1, 2022

swalner-vmware May 31, 2022

mpanchajanya Jun 1, 2022

swalner-vmware May 31, 2022

mpanchajanya Jun 1, 2022

swalner-vmware May 31, 2022

mpanchajanya Jun 1, 2022

miclettej May 31, 2022

mpanchajanya Jun 2, 2022

vuil commented Jun 3, 2022

github-actions bot commented Jun 3, 2022

codecov bot commented Jun 3, 2022 •

edited

Loading

miclettej left a comment

github-actions bot commented Jun 6, 2022

		@@ -47,4 +47,11 @@ hack/providers-sync-tools/**/build
		# Build artifacts related to packages

Refactors Labels Component to Shared TKG Labels #2515

Refactors Labels Component to Shared TKG Labels #2515

Conversation

mpanchajanya commented May 28, 2022 • edited Loading

What this PR does / why we need it

Which issue(s) this PR fixes

Describe testing done for PR

Release note

PR Checklist

Additional information

Special notes for your reviewer

github-actions bot commented May 28, 2022

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

Choose a reason for hiding this comment

vuil commented Jun 3, 2022

github-actions bot commented Jun 3, 2022

codecov bot commented Jun 3, 2022 • edited Loading

Codecov Report

miclettej left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 6, 2022

mpanchajanya commented May 28, 2022 •

edited

Loading

codecov bot commented Jun 3, 2022 •

edited

Loading