block, align-top and wide-input modifiers for mdc-form-field?

[deap82]

Hi!
Would you consider adding modifiers that enables a more traditional way of layouting forms (with a fixed label above the input), or does this go against any of the material design principles? I've made a quick demo here:

https://codepen.io/deap82/pen/GreNPv

Background: We are building a user interface based on material design, but feel that some aspects of the form behaviours (such as the text field animated label) are a bit too advanced (and might be annoying) for our users. Instead we'd like the kind of form interface achieved in my last example in the code pen.

[traviskaufman]

Hey @deap82 ,

I can totally see this as a valid use case. We'd definitely be willing to accept this as part of mdc-form-field if you were to submit a PR for it!

If you're interested in contributing this, could you let me know and also outline each mod class, along with a description of what it does, as an issue comment here? Once that's done I can assign it to you.

[deap82]

Hey @traviskaufman :-)
Ok, that's great. I'll have to look into the contributing guidelines a bit more to get it right.

Could you please specify a bit more what you mean by "outline each mod class". One thing I believe should be noted is that at least some of these modifiers only applies to the non js case of components such as mdc-textfield. Is it that kind of things that should be "outlined"?

Btw, I saw that when you do use the js mdc-textfield, the label goes inside the .mdc-textfield element. Does that mean that the js mdc-textfield should never be wrapped in an .mdc-form-field as this one expects a label as its second child? It seems a bit inconcistent if you do want to use the js version, as well as being consistent with wrapping all fields with .mdc-form-field.

[deap82]

Hello again @traviskaufman,
I've started looking at the contributing guide lines, the source code and dev env setup. And it has raised some questions, to say the least ;-) I've created a fork and branch to play around with.

1. I noticed the form-field had no demo page, so I started creating that. Is someone already working on this? If not, should that be a separate PR before I start adding new features? How can I then continue work while you are reviewing that first PR?

2. When placing two mdc-form-fields after each other, one with an mdc-textfield and one with an mdc-select, I noticed they don't line up nicely. What do you consider the scope of mdc-form-field being, should it adress alignment and spacing between fields, or is it just a wrapper to handle positioning of the label in relation to the input? In my opinion I'd like to be able to have .mdc-form-field elements as siblings and not have to add additional wrappers to take care of alignment and spacing.
   Screenshot:
   

3. In the updated demo for the radio button I saw that each radio button has it's own mdc-form-field wrapper. I can understand the motivation for this, but I also wonder if you would consider it ok to have an mdc-form-field inside another mdc-form field, the case for this would be something like this:

<div class="mdc-form-field">
	<div id="breakfast-radio-buttons">
		<div class="mdc-form-field"><!-- radio and label goes here --></div>
		<div class="mdc-form-field"><!-- radio and label goes here --></div>
		<div class="mdc-form-field"><!-- radio and label goes here --></div>
	</div>
	<label>Please choose your preferred breakfast option</label>
</div>

In your demo for the radio buttons you don't have that kind of header label for the group of radio buttons, but I consider that a very valid scenario where I'd like to use the mdc-form-field as a wrapper as well. The same would go for checkbox list fields, and in some cases even a single checkbox.

4. Is it correct that I have to do a "npm run build" and then restart the dev server ("npm dev run") too see changes I made to the form-field.scss file?

5. In the form-field.scss file I noticed that the existing --align-end modifier is not set up in a way so that both the mdc-form-field and the modifier class are required for the modifier to have any effect. I would have expected this:

.mdc-form-field {
	/.../
	&.mdc-form-field--align-end {
		/.../
	}
}

I'm thinking the current structure could lead to some unexpected behavior for an element that only has the --align-end class... Some insight to the current structure would be interesting.

6. Should one pull request always be exactly one commit? I'm thinking I'd like to separate the demo page of existing features and then each modifier class in to separate commits, but not necessarily separate PRs...

7. Would a commit that only adds/updates a demo page use the "docs" prefix in the commit header? Or "feat"?

8. Regarding:
   "If you've done some experimental work on your branch/fork and committed these via git commit --no-verify, you can rebase them into one correctly-formatted commit before submitting."
   Exactly how do I do this rebase?

[traviskaufman]

Hey @deap82! Apologies for the delay here.

Could you please specify a bit more what you mean by "outline each mod class"

Basically, for every modifier class you want to add, detail the name of the class and what its purpose would be, e.g.

• mdc-form-field--block: Lays out the form control and label vertically rather than horizontally
• mdc-form-field--align-top: Stacks the form label on top of the form control

etc.

Does that mean that the js mdc-textfield should never be wrapped in an .mdc-form-field as this one expects a label as its second child?

I hesitate to use the word never, because I'm sure someone will find a reason to do so 😄 but in most cases, that thinking is correct. The original purpose of mdc-form-field was to assist with RTL rendering of form controls and their corresponding labels. Enhanced text fields handle the automatically. Furthermore, a lot of the functionality of laying out textfields as you've specified can be achieved using standard markup, such as http://codepen.io/traviskaufman/pen/qReLRb. Unfortunately, the spec does not currently offer much guidance in the way of laying out form controls / form elements. This is something that we are actively working on with designers.

I also wonder if you would consider it ok to have an mdc-form-field inside another mdc-form field, the case for this would be something like this

Per the above answer, it's not really something we've considered, and would probably require a bit more thinking from the design system side of things.

Is it correct that I have to do a "npm run build" and then restart the dev server ("npm dev run") too see changes I made to the form-field.scss file?

You should be able to just type npm run dev (not dev run), and the Sass should recompile every time you save and make a change. It may take a few moments to propagate to the browser but no you don't have to continue to manually run npm run build. npm run dev uses webpack-dev-server, which takes care of that for you.

Some insight to the [mdc-form-field--align-end standing along from mdc-form-field] would be interesting.

We follow a modified version of BEM convention for our CSS. mdc-form-field--align-end is a modifier class, thus, it works in conjunction with its block class in order to modify the styles of the element, and should never be used alone. Therefore, a compound class isn't necessary.

Should one pull request always be exactly one commit?

Not necessarily. You can have multiple commits for one PR, but usually we'll Squash and Merge all of those commits into one.

Would a commit that only adds/updates a demo page use the "docs" prefix in the commit header? Or "feat"?

It would use the docs type. However, as I mentioned above we can add/update demos as part of feature work.

Exactly how do I do this rebase?

Check out the rewriting history section in the git book.