Skip to content
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

Some Complex Properties are Inconsistent with rest of API - Discussion #597

Closed
NathanaelA opened this issue Aug 16, 2015 · 4 comments
Closed

Comments

@NathanaelA
Copy link
Contributor

I just noticed this and decided to bring this up as a discussion point. I'm willing to start doing pull requests if you all agree with my logic.

Normally in the Declarative XML you typically do:

<Page><StackLayout><Label/><Label/><Button/></StackLayout></Page>

and all the Labels and Buttons automatically become children of the StackLayout. And the StackLayout becomes a child of the Page component. Nice and clean and precise.
Everyone is used to this; Behind the scenes children are automatically assigned to the parent via the _addChildFromBuilder function.

While messing with the FormattedString I realized it (and several other components) are totally inconsistent with the above clean method of declaring the UI. So lets take the following example:

<Label>
  <Label.formattedText>
    <FormattedString>
       <FormattedString.spans>
         <Span text=”We are” fontSize=”10”/>
         <Span text=”Awesome” fontAttribute=”Bold”/>
       </FormattedString.spans>
    </FormattedString>
  </Label.formattedText>
</Label>

Why do I need to create the <FormattedString.spans> element. We know already that spans is the ONLY element that are children of the FormattedString. So if we added a _addChildFromBuilder to both the Label and FormattedStrings
couldn't we then totally eliminate the <Label.formattedText> and <FormattedString.spans> and make it:

<Label>
    <FormattedString>
         <Span text=”We are” fontSize=”10”/>
         <Span text=”Awesome” fontAttribute=”Bold”/>
    </FormattedString>
</Label>

Wouldn't that be much more consistent with the clean Declarative UI. Since the only valid child component of a Label is a .formattedText and the only valid children of a FormattedString are Spans? Technically, we could even detect if was being passed in as the child, and then dynamically create a FormattedString parent object. So then:

<Label>
         <Span text=”We are” fontSize=”10”/>
         <Span text=”Awesome” fontAttribute=”Bold”/>
</Label>

Would work. 😀 Now, who doesn't think that is WAY simpler to code and read?

Please note; we would still leave the _addArrayFromBuilder on all these components so that you could do the Long hand version (and so all existing code would continue to work). But looking at the code base I don't see any reason why we can't enhance most components that have only one child component type and/or one primary child component type.

@hshristov
Copy link
Contributor

Hi Nathanaela,

you are absolutely right about this. I was considering implementing this change but I wanted to make it more general purpose. What I mean is to decorate with some attribute which is the default property so if you omit <Label.formattedString> it will automatically find on which property this should be applied. Additionally I want the detector (probably component-builder) to detect if this is an array or simple property so there is no need to implement _addArrayFromBuilder.

You suggestion about

<Label>
   <Span text=”We are” fontSize=”10”/>
   <Span text=”Awesome” fontAttribute=”Bold”/>
</Label>

looks really nice. I will research if we can safely integrate it.

Thank you for your continuous support and suggestions on improving NativeScript.

@N3ll N3ll added this to the 1.5 (Under Review) milestone Oct 12, 2015
@enchev
Copy link
Contributor

enchev commented Oct 16, 2015

See this also:#534 and this #518. We should raise error if the content is invalid.

@nsndeck
Copy link

nsndeck commented Oct 21, 2015

Fixed with commit #962.

@nsndeck nsndeck closed this as completed Oct 21, 2015
@nsndeck nsndeck added the done label Oct 21, 2015
@lock
Copy link

lock bot commented Aug 30, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Aug 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants