Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

fix: inline form support #11983

Merged
merged 2 commits into from
Jul 28, 2020
Merged

fix: inline form support #11983

merged 2 commits into from
Jul 28, 2020

Conversation

Splaktar
Copy link
Contributor

PR Checklist

Please check that your PR fulfills the following requirements:

  • The commit message follows our guidelines
  • Tests for the changes have been added or this is not a bug fix / enhancement
  • Docs have been added, updated, or were not required

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Enhancement
[x] Documentation content changes
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

  • md-inline-form support is undocumented (no demo, no docs)
  • there are some duplicate Sass variables
  • there are some styles that users need to adjust, but they aren't associated with Sass variables
  • ng-messages top padding is too small to meet the Material Design spec
  • textareas in inline forms are misaligned
  • radio buttons in inline forms have some layout issues

Issue Number:
Fixes #8712. Closes #8716.

What is the new behavior?

fix(select): support for md-inline-form, more configurable Sass

Create better flexibility for manipulating select while still
following Material Design spec

  • Add md-inline-form support
  • Add alignment math to Sass
  • Support custom margin and padding for md-select
  • Dynamically adjust bottom margin to allow denser layouts

fix(checkbox, date-picker, input, radio-button, select, switch): md-inline-form support

  • add inline form demo
  • add ng-messages to md-select in input errors demo
  • consolidate Sass variables
  • fix padding above ng-messages to align with spec - 8px
  • add docs for md-inline-form
  • fix md-radio-group issues with inline form layout
  • revert some changes to and fix some issues with md-select Sass
    from previous commit

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

Before

Screen Shot 2020-07-28 at 04 36 34

After

Screen Shot 2020-07-28 at 04 20 40

@clshortfuse @Splaktar
fix(select): support for md-inline-form, more configurable SCSS
96e6949 
Create better flexibility for manipulating select while still
following Material Design spec

 - Add `md-inline-form` support
 - Add alignment math to SCSS
 - Support custom margin and padding for `md-select`
 - Dynamically adjust bottom margin to allow denser layouts

Fixes #8712. Closes #8716.
@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added the cla: no PR author needs to sign Google's CLA: https://opensource.google.com/docs/cla/ label Jul 28, 2020
@Splaktar
Copy link
Contributor Author

@clshortfuse can you please add your @googlebot I consent.?

Caretaker: Previous consent was granted in PR #8716.

@Splaktar Splaktar added the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Jul 28, 2020
@Splaktar Splaktar self-assigned this Jul 28, 2020
@Splaktar Splaktar added this to the 1.2.0 milestone Jul 28, 2020
@Splaktar Splaktar added CSS: High Density ui: CSS P3: important Important issues that really should be fixed when possible. labels Jul 28, 2020
@Splaktar
fix(checkbox, date-picker, input, radio-button, select, switch): md-i…
04a4612 
…nline-form support

- add inline form demo
- add `ng-messages` to `md-select` in input errors demo
- consolidate SCSS variables
- fix padding above `ng-messages` to align with spec - `8px`
- add docs for `md-inline-form`
- fix `md-radio-group` issues with inline form layout
- revert some changes to and fix some issues with `md-select` SCSS
  from previous commit
@Splaktar Splaktar force-pushed the inline-form-support branch from 73eb4da to 04a4612 Compare July 28, 2020 09:20
Splaktar
Splaktar commented Jul 28, 2020
View reviewed changes
src/components/input/input.spec.js
@@ -916,8 +913,6 @@ describe('md-input-container directive', function() {
createAndAppendElement('rows="5"');
ngTextarea.val('1\n2\n3\n4\n5\n6\n7');
ngTextarea.triggerHandler('input');
expect(textarea.rows).toBe(7);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea why this is returning 6 instead of 7, but it's not an critical check in the test and manual testing shows that a textarea with this value has 7 rows.

@Splaktar Splaktar requested a review from wagnermaciel July 28, 2020 09:30
@Splaktar Splaktar added pr: merge ready This PR is ready for a caretaker to review and removed in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs labels Jul 28, 2020
@clshortfuse
Copy link
Contributor

@googlebot I consent.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ and removed cla: no PR author needs to sign Google's CLA: https://opensource.google.com/docs/cla/ labels Jul 28, 2020
@Splaktar Splaktar requested review from jelbourn, andrewseguin and mmalerba and removed request for wagnermaciel July 28, 2020 20:57
mmalerba
mmalerba approved these changes Jul 28, 2020
View reviewed changes
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Splaktar Splaktar added the pr: lgtm This PR has been approved by the reviewer label Jul 28, 2020
@Splaktar Splaktar merged commit b3e9ffe into master Jul 28, 2020
@Splaktar Splaktar deleted the inline-form-support branch July 28, 2020 22:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mmalerba mmalerba mmalerba approved these changes

Assignees

@Splaktar Splaktar

Labels
cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ CSS: High Density P3: important Important issues that really should be fixed when possible. pr: lgtm This PR has been approved by the reviewer pr: merge ready This PR is ready for a caretaker to review ui: CSS
Projects
None yet
Milestone
1.2.0
Development

Successfully merging this pull request may close these issues.

select: margin is outside of toolbar
4 participants
@Splaktar @googlebot @clshortfuse @mmalerba