feat(input): option to imperatively float placeholder #2585

devversion · 2017-01-10T13:09:15Z

Refactors the [floatingPlaceholder] input to be able to specifiy whether the label should always float or not.

There are three options for the floatPlaceholder input binding now

If set to "always", the placeholder will always float
If set to "never", the placeholder will never float
If set to "auto", the placeholder will float if text is entered.

Closes #2466

R: @kara @jelbourn

jelbourn · 2017-01-12T00:23:50Z

@kara does this cover what you need for autocomplete?
@mmalerba can you review the input changes?

mmalerba

I'd like to talk to Kara first and see if this is really necessary.

mmalerba · 2017-01-12T17:37:02Z

src/lib/input/input-container.spec.ts

+    let fixture = TestBed.createComponent(MdInputContainerWithDynamicPlaceholder);
+    fixture.detectChanges();
+
+    let inputEl = fixture.debugElement.query(By.css('input'));


I realize this might just be copied from other tests, but shouldn't they either both have .nativeElement or both not have it? Or if we really want them to be different call the second one labelNativeEl

mmalerba · 2017-01-12T18:59:24Z

So it looks like Kara is able to do the autocomplete without this and I would rather not make this change. I plan to eventually make this type of functionality possible by allowing things other than native inputs to be placed inside md-input-container. It would then be up to that custom element to decide when it's empty which will govern whether the label is floating or not

devversion · 2017-01-12T19:01:07Z

@mmalerba I understand. From issue #2466 I thought it would be necessary for the autocomplete. Let's close this PR then.

mmalerba · 2017-01-14T22:45:29Z

src/lib/input/input-container.ts

-  get floatingPlaceholder(): boolean { return this._floatingPlaceholder; }
-  set floatingPlaceholder(value) { this._floatingPlaceholder = coerceBooleanProperty(value); }
-  private _floatingPlaceholder: boolean = true;
+  get floatingPlaceholder() { return this._floatingPlaceholder; }


nit: I would prefer floatPlaceholder it reads better with always and never "always float placeholder", "never float placeholder"

Yeah sounds more imperatively.

mmalerba · 2017-01-14T22:49:43Z

src/lib/input/input-container.ts

+    }
+    this._floatingPlaceholder = value || 'auto';
+  }
+  private _floatingPlaceholder: MD_INPUT_PLACEHOLDER_TYPES = 'auto';


I think we normally name types like classes, maybe just FloatPlaceholder

mmalerba · 2017-01-14T23:03:50Z

src/lib/input/input-container.ts

-  private _floatingPlaceholder: boolean = true;
+  get floatingPlaceholder() { return this._floatingPlaceholder; }
+  set floatingPlaceholder(value: MD_INPUT_PLACEHOLDER_TYPES) {
+    if (value && MD_INPUT_PLACEHOLDER_VALUES.indexOf(value) === -1) {


I would be fine with just not validating this and treating anything other than 'always' and 'never' as auto, up to you though

👍 - Notice that invalid values for the floatPlaceholder attribute will automatically fallback to auto due to the getters here

mmalerba · 2017-01-14T23:04:33Z

src/lib/input/input-container.ts

  MdInputContainerPlaceholderConflictError,
  MdInputContainerDuplicatedHintError,
-  MdInputContainerMissingMdInputError
+  MdInputContainerMissingMdInputError, MdInputContainerFloatingPlaceholderInvalidError


nit: new line

mmalerba · 2017-01-18T18:13:21Z

lgtm

andrewseguin · 2017-01-26T23:03:34Z

Can you rebase?

devversion · 2017-01-27T18:21:50Z

@andrewseguin Done.

kara · 2017-01-31T22:27:05Z

@devversion Can you rebase? Again. Sorry.

Refactors the `[floatingPlaceholder]` input to be able to specifiy whether the label should always float or not. There are three options for the `floatingPlaceholder` input binding now - If set to `true`, the placeholder will *always* float - If set to `false`, the placeholder will *never* float - If set to `null`, the placeholder will float if text is entered. Closes angular#2466

devversion · 2017-02-01T06:31:27Z

@kara No problem. Done

andrewseguin · 2017-02-10T00:26:05Z

Heads up that this is a breaking change for people using the [floatingPlaceholder] input. Can you add a BREAKING CHANGE to your PR description?

angular-automatic-lock-bot · 2019-09-06T08:02:07Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

devversion added the pr: needs review label Jan 10, 2017

devversion requested review from jelbourn and kara January 10, 2017 13:09

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jan 10, 2017

jelbourn requested review from mmalerba and removed request for jelbourn January 12, 2017 00:23

devversion assigned mmalerba and kara Jan 12, 2017

devversion removed request for kara and mmalerba January 12, 2017 17:30

mmalerba reviewed Jan 12, 2017

View reviewed changes

devversion closed this Jan 12, 2017

devversion deleted the feat/input-container-placeholder branch January 12, 2017 19:01

devversion restored the feat/input-container-placeholder branch January 13, 2017 22:12

devversion reopened this Jan 13, 2017

devversion assigned devversion and unassigned kara and mmalerba Jan 13, 2017

devversion added in progress This issue is currently in progress and removed pr: needs review labels Jan 13, 2017

devversion force-pushed the feat/input-container-placeholder branch from ab93d56 to 8932ce5 Compare January 14, 2017 15:11

devversion assigned mmalerba and unassigned devversion Jan 14, 2017

devversion added pr: needs review and removed in progress This issue is currently in progress labels Jan 14, 2017

mmalerba reviewed Jan 14, 2017

View reviewed changes

mmalerba added the pr: lgtm label Jan 18, 2017

mmalerba added action: merge The PR is ready for merge by the caretaker and removed pr: needs review labels Jan 18, 2017

This was referenced Jan 19, 2017

feat(select): add floatingPlaceholder option #2571

Merged

fix(autocomplete): placeholder should float while panel is open #2730

Merged

devversion force-pushed the feat/input-container-placeholder branch from ae203c3 to be7db84 Compare January 27, 2017 18:21

devversion force-pushed the feat/input-container-placeholder branch from be7db84 to 6120f8f Compare January 27, 2017 21:20

kara added pr: needs rebase and removed action: merge The PR is ready for merge by the caretaker labels Jan 31, 2017

devversion force-pushed the feat/input-container-placeholder branch from 6120f8f to 5ab0ea7 Compare February 1, 2017 06:02

devversion added 4 commits February 1, 2017 07:08

Change API to accept string values

9a1314e

Single line comment

066c3d6

Address comments

025fae4

devversion force-pushed the feat/input-container-placeholder branch from 5ab0ea7 to 025fae4 Compare February 1, 2017 06:09

devversion added action: merge The PR is ready for merge by the caretaker and removed pr: needs rebase labels Feb 1, 2017

kara merged commit fb0cf8a into angular:master Feb 3, 2017

devversion deleted the feat/input-container-placeholder branch February 3, 2017 06:33

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019

feat(input): option to imperatively float placeholder #2585

feat(input): option to imperatively float placeholder #2585

Uh oh!

Conversation

devversion commented Jan 10, 2017 • edited by kara Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jelbourn commented Jan 12, 2017

Uh oh!

mmalerba left a comment

Choose a reason for hiding this comment

Uh oh!

mmalerba Jan 12, 2017

Choose a reason for hiding this comment

Uh oh!

mmalerba commented Jan 12, 2017

Uh oh!

devversion commented Jan 12, 2017

Uh oh!

mmalerba Jan 14, 2017

Choose a reason for hiding this comment

Uh oh!

devversion Jan 18, 2017

Choose a reason for hiding this comment

Uh oh!

mmalerba Jan 14, 2017

Choose a reason for hiding this comment

Uh oh!

mmalerba Jan 14, 2017

Choose a reason for hiding this comment

Uh oh!

devversion Jan 18, 2017

Choose a reason for hiding this comment

Uh oh!

mmalerba Jan 14, 2017

Choose a reason for hiding this comment

Uh oh!

mmalerba commented Jan 18, 2017

Uh oh!

andrewseguin commented Jan 26, 2017

Uh oh!

devversion commented Jan 27, 2017

Uh oh!

kara commented Jan 31, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devversion commented Feb 1, 2017

Uh oh!

andrewseguin commented Feb 10, 2017

Uh oh!

angular-automatic-lock-bot bot commented Sep 6, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

devversion commented Jan 10, 2017 •

edited by kara

Loading

kara commented Jan 31, 2017 •

edited

Loading