Horizontal stacking of fields #2376

lenriquez · 2021-04-26T22:31:57Z

Signed-off-by: Luis Enriquez [email protected]

What this PR does / why we need it:

Which issue(s) this PR fixes

Fixes Horizontal stacking of fields #2020 Configurable form field width in stepper #2178 Create FormComponent #2223

Special notes for your reviewer:

For #2020
Vertical did not work so it was necessary to ask clarity team
Question to clarity team
https://vmware.slack.com/archives/C0JF8D2LB/p1619628404441100

Basically, there is no easy way to do it, is necessary to add a div with a layout horizontal for every row like is shown here
https://stackblitz.com/edit/typescript-ku7jz4?file=index.html

In order to encode that information on a JSON a new struct was created where all the horizontal field are contained on the attribute field

#2178
The above solution did not work because label use a lot of space
So at the end it was implemented a solution using width

#2223
The solution proposed by @GuessWhoSamFoo basically changed

to this solution

This is how it looks right now

@GuessWhoSamFoo solution help to reduce the amount of code on form

Release note:

release-note

GuessWhoSamFoo · 2021-04-27T23:24:57Z

web/src/app/modules/shared/components/form-view-container/form-view-container.component.html

- <clr-checkbox-wrapper
- *ngFor="let opt of fieldChoices(field); trackBy: trackByFn"
- >
+<cds-form-group control-width="shrink">


I think layout is working as expected and only allowed for radio buttons/checkboxes. It doesn't solve the original issue though

pkg/view/component/form.go

GuessWhoSamFoo · 2021-05-05T06:03:12Z

pkg/view/component/form.go

@@ -117,6 +118,126 @@ func marshalFormField(ff FormField) ([]byte, error) {
 return json.Marshal(&m)
 }

+type FormFieldLayout struct {
+ *BaseFormField
+ value string


This should be something more descriptive like row size.

I am using the values of a form, I don't think I am adding anything extra to this field

GuessWhoSamFoo · 2021-05-05T06:03:36Z

pkg/view/component/form.go

@@ -117,6 +118,126 @@ func marshalFormField(ff FormField) ([]byte, error) {
 return json.Marshal(&m)
 }

+type FormFieldLayout struct {
+ *BaseFormField


Why is this a form field type?

I don't know, I have three solutions for this

Add a boolean field to Base Form Field to see if it should be horizontal or not but this idea has a flaw

Add a int to know the amount of fields going into a row

Create a struct to save that information there

I think none of them are great solutions, but that's why I wanted to talk before implementing this

GuessWhoSamFoo · 2021-05-05T06:43:17Z

pkg/view/component/form.go

+}
+
+func (ff *FormFieldLayout) SetLabel(value string) {
+ ff.label = value


We can intuitively see the problem of making a field layout into a form field - labels and values are forced to be set even though there are largely not relevant in this context.

This is more apparent under UnmarshalJSON with error, name, validators, etc.

GuessWhoSamFoo · 2021-05-05T07:20:53Z

pkg/view/component/form.go

+ return ff.value
+}
+
+func (ff *FormFieldLayout) UnmarshalJSON(data []byte) error {


imo, this is a lot of extra churn just to insert a class:

https://github.com/vmware-tanzu/octant/pull/2376/files#diff-7d8ccf86c27d790d6cb4eb0c8e6aa65294788afdaa9b3076373a07e6a3a867f6R4

I think the preferred way to fix this is to implement the to-do (convert FormField into a struct).

The problem is not the class but rather the div that provides the horizontal layout

lenriquez · 2021-05-20T19:15:46Z

pkg/view/component/form_test.go

@@ -192,7 +97,8 @@ func TestForm_UnmarshalJSON(t *testing.T) {
 var got Form
 require.NoError(t, json.Unmarshal(data, &got))

- assert.Equal(t, form, got)
+ dataGot, err := json.Marshal(&got)
+ assert.JSONEq(t, string(data), string(dataGot))


I am not sure about this, equal is comparing pointers and not values

mklanjsek

Looks pretty good just a few small thing mostly naming

web/src/stories/stepper.stories.mdx

web/package-lock.json

web/angular.json

pkg/view/component/form.go

GuessWhoSamFoo · 2021-06-02T19:47:19Z

web/src/stories/stepper.stories.mdx

@@ -115,3 +140,160 @@ export const StepperStoryTemplate = args => ({

 <h2>Props</h2>
 <Props story = "Stepper component"/>
+
+export const stepperHFDocs = {
+ source: {code: `title = "Example Stepper with Horizontal Form" component.NewStepper()`},


Add a snippet of working code that reflects what is shown

…epper Signed-off-by: Luis Enriquez <[email protected]>

GuessWhoSamFoo reviewed Apr 27, 2021

View reviewed changes

lenriquez force-pushed the issue-2020 branch 3 times, most recently from 66e9795 to 1465387 Compare May 3, 2021 21:25

lenriquez marked this pull request as ready for review May 3, 2021 21:27

lenriquez force-pushed the issue-2020 branch from 1465387 to 1a7ba73 Compare May 4, 2021 20:24

GuessWhoSamFoo suggested changes May 5, 2021

View reviewed changes

lenriquez force-pushed the issue-2020 branch 4 times, most recently from d1fe1d1 to 10b4d7b Compare May 20, 2021 19:06

lenriquez commented May 20, 2021

View reviewed changes

mklanjsek reviewed May 21, 2021

View reviewed changes

lenriquez force-pushed the issue-2020 branch 3 times, most recently from 8df7222 to b00829d Compare June 1, 2021 03:02

mklanjsek approved these changes Jun 1, 2021

View reviewed changes

GuessWhoSamFoo mentioned this pull request Jun 2, 2021

Update plugin and Yeoman generator to 0.21.0, add more recent components from Octant vmware-archive/plugin-library-for-octant#21

Merged

GuessWhoSamFoo reviewed Jun 2, 2021

View reviewed changes

Horizontal stacking of fields and Configurable form field width in st…

331ef55

…epper Signed-off-by: Luis Enriquez <[email protected]>

lenriquez force-pushed the issue-2020 branch from b00829d to 331ef55 Compare June 3, 2021 22:51

GuessWhoSamFoo approved these changes Jun 3, 2021

View reviewed changes

GuessWhoSamFoo merged commit 68df9b7 into vmware-archive:master Jun 3, 2021

This was referenced Jun 8, 2021

Configurable form field width in stepper #2178

Closed

Create FormComponent #2223

Closed

karayim mentioned this pull request Jun 28, 2021

Add Form Component to Storybook #2603

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Horizontal stacking of fields #2376

Horizontal stacking of fields #2376

lenriquez commented Apr 26, 2021 •

edited

Loading

GuessWhoSamFoo Apr 27, 2021

GuessWhoSamFoo May 5, 2021

lenriquez May 10, 2021

GuessWhoSamFoo May 5, 2021

lenriquez May 10, 2021

GuessWhoSamFoo May 5, 2021

GuessWhoSamFoo May 5, 2021

lenriquez May 10, 2021 •

edited

Loading

lenriquez May 20, 2021

mklanjsek left a comment

GuessWhoSamFoo Jun 2, 2021

lenriquez Jun 3, 2021

Horizontal stacking of fields #2376

Horizontal stacking of fields #2376

Conversation

lenriquez commented Apr 26, 2021 • 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

lenriquez May 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mklanjsek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lenriquez commented Apr 26, 2021 •

edited

Loading

lenriquez May 10, 2021 •

edited

Loading