Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

feat(form-field): Add support for space-between #5754

Closed
wants to merge 11 commits into from
Closed

feat(form-field): Add support for space-between #5754

wants to merge 11 commits into from

Conversation

Misiu
Copy link

@Misiu Misiu commented Mar 27, 2020

This PR adds mdc-form-field--space-between class.
fixes: #5747
This allows easily creating this layout:
image

https://jsfiddle.net/Misiu/quzgcsev/25/
Sample code:

<div class="mdc-form-field mdc-form-field--space-between" id="form1">
  <div class="mdc-switch" id="switch1">
    <div class="mdc-switch__track"></div>
    <div class="mdc-switch__thumb-underlay">
      <div class="mdc-switch__thumb"></div>
      <input type="checkbox" id="basic-switch1" class="mdc-switch__native-control" role="switch" aria-checked="false">
    </div>
  </div>
  <label for="basic-switch1">off/on</label>
</div>

I've added support for mdc-form-field--align-end, but I'm not sure how support for RTL languages should look.

Also, this is my first PR in this repo, so sorry for any mistakes.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@Misiu
Copy link
Author

Misiu commented Mar 27, 2020

@googlebot I signed it!

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@Misiu Misiu changed the title Add support for space-between in form field feat(form-field): Add support for space-between Mar 27, 2020
packages/mdc-form-field/_mixins.scss Outdated Show resolved Hide resolved
packages/mdc-form-field/_mixins.scss Outdated Show resolved Hide resolved
packages/mdc-form-field/_mixins.scss Outdated Show resolved Hide resolved
packages/mdc-form-field/_mixins.scss Outdated Show resolved Hide resolved
packages/mdc-form-field/_mixins.scss Outdated Show resolved Hide resolved
Copy link
Contributor

@asyncLiz asyncLiz left a comment

Choose a reason for hiding this comment

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

Nice work! We'll pull it in to do some more testing, but this looks good to me :)

@Misiu
Copy link
Author

Misiu commented Mar 27, 2020

@asyncLiz thanks for the help and for the tips!
If this gets merged then I'll take a look at Web Component next 🙂

packages/mdc-form-field/_mixins.scss Outdated Show resolved Hide resolved
packages/mdc-form-field/_mixins.scss Outdated Show resolved Hide resolved
packages/mdc-form-field/_mixins.scss Outdated Show resolved Hide resolved
packages/mdc-form-field/_mixins.scss Outdated Show resolved Hide resolved
packages/mdc-form-field/_mixins.scss Outdated Show resolved Hide resolved
@asyncLiz
Copy link
Contributor

One final round of reviews to go, getting close :)

@Misiu
Copy link
Author

Misiu commented Mar 31, 2020

@asyncLiz ok, I think it is ready.
I had to change the order in
@include rtl-mixins.reflexive-property(margin, auto, initial);
but I think it works ok now: https://jsfiddle.net/Misiu/u247sya5/3/

Waiting for your review :)

Co-Authored-By: Liz Mitchell <asyncliz@gmail.com>
@Misiu
Copy link
Author

Misiu commented Mar 31, 2020

done :)

packages/mdc-form-field/_mixins.scss Outdated Show resolved Hide resolved
packages/mdc-form-field/_mixins.scss Outdated Show resolved Hide resolved
Misiu and others added 2 commits April 1, 2020 08:11
Co-Authored-By: Liz Mitchell <asyncliz@gmail.com>
Co-Authored-By: Liz Mitchell <asyncliz@gmail.com>
@Misiu
Copy link
Author

Misiu commented Apr 1, 2020

@asyncLiz I wasn't aware IE11 is supported :/
It's in the repo (https://github.com/material-components/material-components-web/blob/master/docs/supported-browsers.md) but I didn't look. Sorry about that.

@asyncLiz
Copy link
Contributor

asyncLiz commented Apr 1, 2020

No worries! That's my fault for suggesting the initial without realizing it. Thanks for sticking with this 🙂

@Misiu
Copy link
Author

Misiu commented Apr 1, 2020

Glad I could help 🙂

@asyncLiz
Copy link
Contributor

asyncLiz commented Apr 1, 2020

Hm our bots are going a little bit crazy. Everything worked, except copybara created a new PR at #5759 and merged that instead of this one... 🤦‍♀️

Not entirely sure how that happened, but the change is in master now. I'll close this and comment on the other PR to refer to this one for the history.

@asyncLiz
Copy link
Contributor

asyncLiz commented Apr 1, 2020

For MWC, feel free to go ahead and open a PR to get the process started. Hopefully it'll be a little smoother.

@Misiu
Copy link
Author

Misiu commented Apr 3, 2020

@asyncLiz I've tried adding this to MWC, but I got stuck on setup. I work on Windows 10 machine and I can't run any npm scripts. The issue was reported here: material-components/material-web#459.
Could you take a look at it? Thanks 🙂

@asyncLiz
Copy link
Contributor

asyncLiz commented Apr 6, 2020

Sure thing, should be a trivial thing to add

@Misiu
Copy link
Author

Misiu commented Apr 8, 2020

@asyncLiz thank you.
What I had in mind was that bug (material-components/material-web#459), but I'm glad this feature got added.
As I wrote before I'm a windows user and because of that bug I can't even start dev environment to contribute.

@asyncLiz
Copy link
Contributor

asyncLiz commented Apr 8, 2020

No worries. I went ahead and implemented it because I knew I'd need to do some work anyway to get screenshot tests up to date. It'd also have been faster than the windows bug, which was lower priority 😊

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support space-between in form field
3 participants