New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use custom directive to style the hint text instead of utility function #431

Open
SamVerschueren opened this Issue Jul 5, 2016 · 9 comments

Comments

Projects
None yet
4 participants
@SamVerschueren
Contributor

SamVerschueren commented Jul 5, 2016

I believe this section where the tutorial styles the hint text of the input fields is a very nice situation where custom directives can be introduced to the user. I think a directive is actually also more suited for that.

This is how I achieved the same result but in a more angular 2 way.

hint-color.base.directive.ts

Might as well be repeated in both the iOS and Android implementation, but just showing how I did it.

import {ElementRef, Input} from '@angular/core';
import {TextField} from 'ui/text-field';
import {Color} from 'color';

export abstract class HintColorBaseDirective {

    constructor(
        private _el: ElementRef
    ) { }

    @Input() set hintColor(value: string) {
        const textField = <TextField>this._el.nativeElement;

        this.setColor(textField, new Color(value));
    }

    protected abstract setColor(view: TextField, color: Color);
}

hint-color.directive.android.ts

import {Directive, ElementRef, Input} from '@angular/core';
import {TextField} from 'ui/text-field';
import {Color} from 'color';
import {HintColorBaseDirective} from './hint-color.base.directive';

@Directive({
    selector: '[hintColor]'
})
export class HintColorDirective extends HintColorBaseDirective {

    constructor(el: ElementRef) {
        super(el);
    }

    protected setColor(view: TextField, color: Color) {
        view.android.setHintTextColor(color.android);
    }
}

hint-color.directive.ios.ts

import {Directive, ElementRef, Input} from '@angular/core';
import {TextField} from 'ui/text-field';
import {Color} from 'color';
import {HintColorBaseDirective} from './hint-color.base.directive';

declare const NSAttributedString: any;
declare const NSDictionary: any;
declare const NSForegroundColorAttributeName: any;

@Directive({
    selector: '[hintColor]'
})
export class HintColorDirective extends HintColorBaseDirective {

    constructor(el: ElementRef) {
        super(el);
    }

    protected setColor(view: TextField, color: Color) {
        let dictionary = new NSDictionary(
            [color.ios],
            [NSForegroundColorAttributeName]
        );

        view.ios.attributedPlaceholder = NSAttributedString.alloc().initWithStringAttributes(view.hint, dictionary);
    }
}

login.component.ts

Add HintColorDirective to the directives list of the component

const {HintColorDirective} = require('../../utils/hint-color.directive');

@Component({
    ...
    directives: [HintColorDirective]
})

It's not possible to use ES6 module imports with import {HintColorDirective} from '...'; because hint-color.directive.ts does not exist and the TypeScript compiler will fail on that one. So the only way (I believe) this can be solved is by using the good old require :).

login.html

The only thing we have to do is add a hintColor attribute to our TextField component.

<TextField
    hint="Email Address" 
    keyboardType="email"
    autocorrect="false" 
    autocapitalizationType="none"
    [(ngModel)]="user.email"
    [hintColor]="isLoggingIn ? '#ACA6A7' : '#C4AFB4'">
</TextField>

<TextField
    hint="Password"
    secure="true"
    [(ngModel)]="user.password"
    [hintColor]="isLoggingIn ? '#ACA6A7' : '#C4AFB4'">
</TextField>

A major benefit on this one is that the view is responsible for the styling and rendering of the component. This way you decouple that from the login component.

If you guys agree, I'm always willing to do a PR and try to change the docs with this implementation instead.

Any feedback is more then welcome!

@tjvantoll

This comment has been minimized.

Show comment
Hide comment
@tjvantoll

tjvantoll Jul 5, 2016

Contributor

Hey @SamVerschueren,

I love this idea. I’ve always thought the way I implemented this functionality was icky, but my lack of in-depth Angular 2 knowledge has kept me from implementing this code the Angular way.

One suggestion: please use just one file for the directive and avoid the .android. and .ios.thing. For the tutorials I like keeping the number files to a minimum, and also I like using this section as a way to teach how to use if statements to fork platform code. Using one file should also remove the need to use require().

I’d love a PR and would be happy to incorporate the changes. Just keep the current structure; implement the directive in 6.1; and add in the Android code in 6.2. Ping me in the PR.

I’ll also ping @vakrilov here, in case he wants to add anything about the Angular code in particular.

Contributor

tjvantoll commented Jul 5, 2016

Hey @SamVerschueren,

I love this idea. I’ve always thought the way I implemented this functionality was icky, but my lack of in-depth Angular 2 knowledge has kept me from implementing this code the Angular way.

One suggestion: please use just one file for the directive and avoid the .android. and .ios.thing. For the tutorials I like keeping the number files to a minimum, and also I like using this section as a way to teach how to use if statements to fork platform code. Using one file should also remove the need to use require().

I’d love a PR and would be happy to incorporate the changes. Just keep the current structure; implement the directive in 6.1; and add in the Android code in 6.2. Ping me in the PR.

I’ll also ping @vakrilov here, in case he wants to add anything about the Angular code in particular.

@SamVerschueren

This comment has been minimized.

Show comment
Hide comment
@SamVerschueren

SamVerschueren Jul 5, 2016

Contributor

Will try to do one in the upcoming days, thanks for the feedback!

Contributor

SamVerschueren commented Jul 5, 2016

Will try to do one in the upcoming days, thanks for the feedback!

@vakrilov

This comment has been minimized.

Show comment
Hide comment
@vakrilov

vakrilov Jul 6, 2016

Member

Definitely good idea! Extracting platform specific code into separate directives (you can even put them in a plugin) is a great way to keep the code clean.

I'm not sure if there is a way to define all in one file as you have to have different implementations of the same class depending on the platform. You can, of course skip the base class, and do single class with an if-platform check inside the setColor.

Personally, I would prefer multi-file approach. About the import/require issue - it can be solved by creating a hint-color.directive.d.ts file with the definition of the HintColorDirective directive class. It will be used compile-time so that tsc is happy.

Member

vakrilov commented Jul 6, 2016

Definitely good idea! Extracting platform specific code into separate directives (you can even put them in a plugin) is a great way to keep the code clean.

I'm not sure if there is a way to define all in one file as you have to have different implementations of the same class depending on the platform. You can, of course skip the base class, and do single class with an if-platform check inside the setColor.

Personally, I would prefer multi-file approach. About the import/require issue - it can be solved by creating a hint-color.directive.d.ts file with the definition of the HintColorDirective directive class. It will be used compile-time so that tsc is happy.

@SamVerschueren

This comment has been minimized.

Show comment
Hide comment
@SamVerschueren

SamVerschueren Jul 6, 2016

Contributor

Personally, I would prefer multi-file approach.

I prefer that as well because you clear split up logic for one platform or another.

it can be solved by creating a hint-color.directive.d.ts file

Will definitely try that!

Contributor

SamVerschueren commented Jul 6, 2016

Personally, I would prefer multi-file approach.

I prefer that as well because you clear split up logic for one platform or another.

it can be solved by creating a hint-color.directive.d.ts file

Will definitely try that!

@tjvantoll

This comment has been minimized.

Show comment
Hide comment
@tjvantoll

tjvantoll Jul 6, 2016

Contributor

Personally, I would prefer multi-file approach.

I prefer that as well because you clear split up logic for one platform or another.

Ok I’ve been out voted 😄 . Go ahead and move forward with whatever you think is the cleanest approach. I think this could work well because it also gives us a chance to talk about .d.ts files, which the tutorial currently kind of glosses over.

Contributor

tjvantoll commented Jul 6, 2016

Personally, I would prefer multi-file approach.

I prefer that as well because you clear split up logic for one platform or another.

Ok I’ve been out voted 😄 . Go ahead and move forward with whatever you think is the cleanest approach. I think this could work well because it also gives us a chance to talk about .d.ts files, which the tutorial currently kind of glosses over.

@tjvantoll

This comment has been minimized.

Show comment
Hide comment
@tjvantoll

tjvantoll Aug 10, 2016

Contributor

Hey @SamVerschueren,

Just out of curiosity have you made any progress on this? As an fyi, in case you have active changes, I’m going to be pushing out some updates to update the guide to Angular RC4 later today and tomorrow.

Contributor

tjvantoll commented Aug 10, 2016

Hey @SamVerschueren,

Just out of curiosity have you made any progress on this? As an fyi, in case you have active changes, I’m going to be pushing out some updates to update the guide to Angular RC4 later today and tomorrow.

@SamVerschueren

This comment has been minimized.

Show comment
Hide comment
@SamVerschueren

SamVerschueren Aug 10, 2016

Contributor

I started rewriting that piece but just do your thing. I'll adjust ;).

Contributor

SamVerschueren commented Aug 10, 2016

I started rewriting that piece but just do your thing. I'll adjust ;).

@sarvagayatri

This comment has been minimized.

Show comment
Hide comment
@sarvagayatri

sarvagayatri Sep 8, 2016

i am getting error on using same code

JS: EXCEPTION: Error in login/login.html:4:55
JS: ORIGINAL EXCEPTION: TypeError: Cannot read property 'setHintTextColor' of un
defined
JS: ORIGINAL STACKTRACE:
JS: TypeError: Cannot read property 'setHintTextColor' of undefined
JS: at HintColorDirective.setColor (/data/data/org.nativescript.Practise/fil
es/app/utils/hint-color-directive.js:10:21)
JS: at HintColorDirective.Object.defineProperty.set [as hintColor](/data/da
ta/org.nativescript.Practise/files/app/utils/hint-color-base.js:11:18)
JS: at DebugAppView._View_Login0.detectChangesInternal (Login.template.js:16
0:44)
JS: at DebugAppView.AppView.detectChanges (/data/data/org.nativescript.Pract
ise/files/app/tns_modules/@angular/core/src/linker/view.js:243:14)
JS: at DebugAppView.detectChanges (/data/data/org.nativescript.Practise/file
s/app/tns_modules/@angular/core/src/linker/view.js:348:44)
JS: at DebugAppView.AppView.detectViewChildrenChanges (/data/data/org.native
script.Practise/files/app/tns_modules/@angular/core/src/linker/view.js:269:19)
JS: at DebugAppView._View_Login_Host0.detectChangesInternal (Login.template.
js:31:8)
JS: at DebugAppView.AppView.detectChanges (/data/data/org.nativescript.Pract
ise/files/app/tns_modules/@angular/core/src/linker/view.js:243:14)
JS: at DebugAppView.detectChanges (/data/data/org.nativescript.Practise/file
s/app/tns_modules/@angular/core/src/linker/view.js:348:44)
JS: at DebugAppView.AppView.detectContentChildrenChanges (/data/data/org.nat
ivescript.Practise/files/app/tns_modules/@angular/core/src/linker/view.js:261:19

sarvagayatri commented Sep 8, 2016

i am getting error on using same code

JS: EXCEPTION: Error in login/login.html:4:55
JS: ORIGINAL EXCEPTION: TypeError: Cannot read property 'setHintTextColor' of un
defined
JS: ORIGINAL STACKTRACE:
JS: TypeError: Cannot read property 'setHintTextColor' of undefined
JS: at HintColorDirective.setColor (/data/data/org.nativescript.Practise/fil
es/app/utils/hint-color-directive.js:10:21)
JS: at HintColorDirective.Object.defineProperty.set [as hintColor](/data/da
ta/org.nativescript.Practise/files/app/utils/hint-color-base.js:11:18)
JS: at DebugAppView._View_Login0.detectChangesInternal (Login.template.js:16
0:44)
JS: at DebugAppView.AppView.detectChanges (/data/data/org.nativescript.Pract
ise/files/app/tns_modules/@angular/core/src/linker/view.js:243:14)
JS: at DebugAppView.detectChanges (/data/data/org.nativescript.Practise/file
s/app/tns_modules/@angular/core/src/linker/view.js:348:44)
JS: at DebugAppView.AppView.detectViewChildrenChanges (/data/data/org.native
script.Practise/files/app/tns_modules/@angular/core/src/linker/view.js:269:19)
JS: at DebugAppView._View_Login_Host0.detectChangesInternal (Login.template.
js:31:8)
JS: at DebugAppView.AppView.detectChanges (/data/data/org.nativescript.Pract
ise/files/app/tns_modules/@angular/core/src/linker/view.js:243:14)
JS: at DebugAppView.detectChanges (/data/data/org.nativescript.Practise/file
s/app/tns_modules/@angular/core/src/linker/view.js:348:44)
JS: at DebugAppView.AppView.detectContentChildrenChanges (/data/data/org.nat
ivescript.Practise/files/app/tns_modules/@angular/core/src/linker/view.js:261:19

@sarvagayatri

This comment has been minimized.

Show comment
Hide comment
@sarvagayatri

sarvagayatri Sep 8, 2016

While debugging i got error in hint-color.directive.android.ts ,
android is undefined in code, even view object is getting loaded properly

protected setColor(view: TextField, color: Color) {
        view.android.setHintTextColor(color.android);
    }

sarvagayatri commented Sep 8, 2016

While debugging i got error in hint-color.directive.android.ts ,
android is undefined in code, even view object is getting loaded properly

protected setColor(view: TextField, color: Color) {
        view.android.setHintTextColor(color.android);
    }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment