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

Feature suggestions and a question #41

Closed
jbmonroe opened this issue Apr 29, 2017 · 28 comments
Closed

Feature suggestions and a question #41

jbmonroe opened this issue Apr 29, 2017 · 28 comments
Assignees
Labels

Comments

@jbmonroe
Copy link

Features:

  1. Make Moon#set chainable
  2. When data has member field1, make moonInstance.field1 a property, triggering Moon#set on assignment. This need not apply to arbitrarily complex data names. (I'm going to experiment with this a bit; I've already done a POC regarding the addition of arbitrary properties to a page framework.)

Item 2 brings me to a question: if my application has (for example) flight segments to be rendered

const trip = {
    segments : [
       { origin:"MIA",dest:"MCO",date:"09SEP17",departs:"0800",arrives:"0852"},
       { origin:"MCO",dest:"ATL",date:"09SEP17",departs:"1020",arrives:"1115"},
       // etc.
    ]
};

then what does the HTML look like to render that in Moon? (I couldn't make it work the way I wanted to in Vue, and mustaches don't seem to lend themselves to it.) Overall these frameworks seem oriented toward single-field renders or (at best) single-column data amenable to UL rendering. Can't quite get my head around complex data rendering for these things.

@kbrsh kbrsh self-assigned this Apr 29, 2017
@kbrsh kbrsh added the feature label Apr 29, 2017
@kbrsh kbrsh added this to the v0.10.0 milestone Apr 29, 2017
@kbrsh
Copy link
Owner

kbrsh commented Apr 29, 2017

Thanks for the request!

  1. I'll add this in v0.10.0
  2. I can turn this into a plugin if you'd like, and it will simply alias data to the Moon instance, and setting them will call set. Be careful though, attaching getters/setters to deep data structures can slow down the initial startup of your app.

Regarding the question, you can simply use m-for and access data from the segments, like:

<div id="app">
  <ul>
    <li m-for="segment in {{trip.segments}}">
      <p>Origin: {{segment.origin}}</p>
      <p>Destination: {{segment.dest}}</p>
      <p>Date: {{segment.date}}</p>
      <p>Departs: {{segment.departs}}</p>
      <p>Arrives: {{segment.arrives}}</p>
    </li>
  </ul>
</div>
new Moon({
  el: "#app",
  data: {
    trip: {
      segments : [
        { origin:"MIA",dest:"MCO",date:"09SEP17",departs:"0800",arrives:"0852"},
        { origin:"MCO",dest:"ATL",date:"09SEP17",departs:"1020",arrives:"1115"}
      ]
    }
  }
});

Here is a jsFiddle link

@jbmonroe
Copy link
Author

Excellent...I'm going to work up a POC for the airline thing. (It's relevant to work.) I definitely wouldn't attach get/set to deep items, although there's a cheat (kind of):

    class ExportSelfMember
    {
        constructor ()
        {
            const self = this;
            this._array = [0,1,2,3,4,5,6,7,8,9];
            Object.defineProperty(this,'array', {
                get : function () {return self._array;}
            });
        }
    }

    let a = new ExportSelfMember();

    a.array[3] --> 3

The experiment I did was in gathering all specially-marked items in a page via querySelectAll, and adding them as members of an overarching Page.View object.

// Presentation field                  Input form field
    myPage.view.customerName = newCustomerName.value;

then queued an update for the page, which ran in either an idle callback or animation callback. I didn't get far enough into it to see how well it scaled, but it was part of a plan that would let the page designers do the layouts and mark the active fields with data- attributes, after which time the coders would come in and connect the dots, so to speak.

It's only the kind of thing I'd do with cheap look-ups in the model.

@kbrsh
Copy link
Owner

kbrsh commented Apr 29, 2017

Cool!

Here's a little plugin that does a shallow run through of the data, and proxies it to the instance, for all Moon instances automatically:

var MoonTracked = {
  init: function(Moon) {
    var init = Moon.prototype.init;
    Moon.prototype.init = function() {
      var data = this.$data;
      var self = this;

      var track = function(key) {
        Object.defineProperty(self, key, {
          get: function(key) {
            return data[key];
          },
          set: function(val) {
            self.set(key, val);
          }
        });
      }

      for(var key in data) {
        track(key);
      }

      init.apply(this, arguments);
    }
  }
}

Moon.use(MoonTracked);

var app = new Moon({
  el: "#app",
  data: {
    msg: "Hello Moon!"
  }
});

app.msg = "Changed!";
<div id="app">
  <h1>{{msg}}</h1>
</div>

Here is a jsFiddle

@jbmonroe
Copy link
Author

jbmonroe commented Apr 29, 2017

Thanks--I'll check that out.

Meanwhile, it doesn't like this:


            <tbody m-for="pax in {{trip.passengers}}">
                <tr>
                    <td>{{pax.name}}</td>
                    <td>{{pax.ffn}}</td>
                    <dummy m-for="seat in {{pax.seats}}">
                        <td>{{seat}}</td>
                    </dummy>
                </tr>
            </tbody>
            data : {
                msg : "Itinerary for Alice Kramden",
                trip : {
                    passengers : [
                        {
                            name : "Kramden/Alice",
                            ffn : "987654321",
                            seats : ['6A','7B']
                        }
                    ],
                    segments : [
                        { number: "745", origin:"MIA",dest:"MCO",date:"09SEP17",departs:"0800",arrives:"0852", cos : "First"},
                        { number: "611", origin:"MCO",dest:"ATL",date:"09SEP17",departs:"1020",arrives:"1115", cos : "Coach"},
                    ]
                }
            }

(unknown) Uncaught TypeError: Cannot read property 'seats' of undefined

Does the inner m-for not have a context for pax? The examples don't have any nested loops, but neither do they seem to be prohibited.

The data from the server will always have seats as an array embedded in another object, so access to previous m-for temporary handles would be necessary since the seats can't be referenced without context to another iterated entity. (I haven't looked at the Moon code to see if the m-for proxies are kept in a stack. That would help resolve the issues.)

Because the table column count varies based on the number of itinerary segments, I have to put in a tag pair (mimicking the very convenient ) to support writing a variable number of instances into the row. That makes me vaguely uneasy, and it probably wouldn't validate, but it seems to be the only thing I can do.

(Yeah, tables. Passe', but the accessibility requirements set forth by the Americans with Disabilities Act pretty much sum up to "novel approaches to layout are nice for everyone else, but the cool kids use tables for accessible content to avoid confusing screen reader plug-ins").

[If this turns out to be a typo on my part I'm going to be very annoyed at myself.]

@kbrsh
Copy link
Owner

kbrsh commented Apr 29, 2017

Moon does allow nested m-for, and they are used in the dbMonster benchmark.

I think this is because of the table. Before going through Moon's compiler, the HTML is normalized by the browser, and it is removing the dummy element since the HTML5 spec only allows certain elements to be inside of tr.

As an alternative, try <td m-for="seat in {{pax.seats}}">{{seat}}</td>.

Also, as far as I know, tbody should be in a table once, not multiple times (via m-for). Instead, you should have multiple rows/columns.

@jbmonroe
Copy link
Author

jbmonroe commented May 2, 2017

Yeah, I think the reason I went with putting the m-for in the tbody is that I'm thinking like a programmer: "the m-for is repeating what's in the tbody, not the tbody." (The m-for should be on the tr, but that's not what occurred to me the first time. But I see a 'for' and I'm all about the loop body--which isn't what happens here. Have to kick my brain into a different gear for the markup.)

At least it wasn't a typo.

The other reason I was thinking that way is that I wrote a server-side C++ framework 16 years go in which I added new tags to html (<repeat>, <if> <else>) that extracted their contents and did whatever was appropriate based on conditions expressed as attributes. With my tags, only the contents were repeated.

<!-- passenger was expressed in an outer loop -->
<repeat range="passenger[%d].seats'>
   <td>{{[passenger[%d].seat[%d]}}</td>
</repeat>

(The framework kept track of the indexing internally.)

@jbmonroe
Copy link
Author

jbmonroe commented May 2, 2017

alice-to-the-moon

@jbmonroe
Copy link
Author

jbmonroe commented May 2, 2017

It's not pretty but pretty wasn't the goal. Thanks!

@kbrsh
Copy link
Owner

kbrsh commented May 3, 2017

Love it! 🙌

@jbmonroe
Copy link
Author

jbmonroe commented May 3, 2017

So, another question (because they don't end, right?): if I try to render a seat map, I'm going to have to apply all sorts of styles to the element based on class of service, taken/open, travel partner, wing, exit row, etc. That seems a bit of a push to implement with m-if and otherwise static markup. It seems like a callback might be required to apply classes to the to-be-rendered element. I didn't see any support for callback functions on a per-element basis in the docs, but I could have missed something.

@kbrsh
Copy link
Owner

kbrsh commented May 3, 2017

There is a cleaner way to conditionally apply classes to an element via m-literal. If your styles are just applied by different classnames, you can pass an object to class with all the conditions needed for it to be rendered, like:

<h1 id="app" class="{class1: {{condition1}}, class2: {{condition2}}}
new Moon({
  el: "#app",
  data: {
    condition1: false
    condition2: true
  }
})

See the m-literal API reference.

@jbmonroe
Copy link
Author

jbmonroe commented May 3, 2017

Yeah, except that it has to be done for every seat, and I didn't figure to have to reset the data for every seat. If conditions 1 and 2 could be functions from which the true/false result was captured. (Likewise, adding an extra field for 156-300 seats would be computationally debilitating.)

data : {
  passengers : [],
  segments : [],
  seatmap : [
      cabin : [
         map : [ 'A','B','C',' ','D','E','F'],
         row : [
            number : '4',
            seats: [
                {
                     wing : true,
                     exit : true,
                     taken : false,
                     premium : true,
                    cost : 25
                },...]
           ]
      ]
  ]
}

One usually shows only a single cabin at a time, so that makes it easier. I already have code that can compose a seatmap with straight-up DOM--so I could hide it behind a class instance that just took a DOM element ID and used it--but I thought I'd try to do a performance test using Moon. One means of building the page would be less confusing for the team--especially incoming members.

@kbrsh
Copy link
Owner

kbrsh commented May 3, 2017

Ahh, I see.

In v0.10.0, there will be different scoping (there will be no mustache templates inside of directives and m-literal, they will be evaluated and compiled as expressions).

For example, in v0.10.0 you will do something like:

<div id="app">
  <h1 id="{{dynamic}}" m-literal:class="{class1: condition1, class2: condition2}">Hello Moon!</h1>
  <p>{{1 + 2}}</p>
  <ul m-if="list.length > 0">
    <li m-for="item in items">{{item}}</li>
  </ul>
</div>

As you can see, when using m-literal, m-if, m-for, or any directive (including custom directives), the inner item will be evaluated as an expression. Any mustache templates (as seen in the p tag) will also be able to be any valid Javascript expression.

This will allow you to use a custom method to decide what class should belong to which seat. You can do something like (in v0.10.0):

<div id="app">
  <ul>
    <li m-for="seat in seats" class="{{ getClass(seat) }}">
      {{seat}}
    </li>
  </ul>
</div>
new Moon({
  el: "#app",
  data: {
    seats: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
  },
  methods: {
    getClass: function(seat) {
      if(seat > 5) {
        return 'red';
      } else {
        return 'blue';
      }
    }
  }
});
.red {
  color: red;
}

.blue {
  color: blue;
}

You can use this custom method to dynamically return a class name (or false to indicate no class) based on the seat. Luckily, v0.10.0 should be coming out this Friday 😄

@jbmonroe
Copy link
Author

jbmonroe commented May 3, 2017

That would be exactly the thing. Except (as a C++ programmer) functions with variant return types make me nervous (and if TypeScript is ever involved, it will be unhappy as well). Empty strings would work as well as false and maintain return type integrity. I've seen libraries that had single functions returning a number, a string, a Boolean, and undefined, and I can't figure out how a caller would know what to expect. I either don't use those libraries, or I rewrite the code to have a single, sensible return type. (I don't mind aggregate return types, but no POD variant returns.)

@kbrsh
Copy link
Owner

kbrsh commented May 3, 2017

Totally get it, an empty string will work as well 😉

@kbrsh kbrsh removed this from the v0.10.0 milestone May 20, 2017
@kbrsh
Copy link
Owner

kbrsh commented May 20, 2017

I removed the milestone for making .set chainable, as I don't really feel that is a good solution to changing multiple items.

Maybe add a new method that can merge an object? (Like React's setState)

Let me know what you think.

@jbmonroe
Copy link
Author

It comes down to code size. If one compiled JavaScript, then the source code size doesn't matter. Minification isn't going to resolve the size issue for multiple calls, either, because one still carries the overhead of the smallest possible variable name, plus the de-reference. It seems to work well enough for JQuery.

Plying me with React will get you nowhere. I don't like React. :) That's why I'm interested in Moon.

One thing lacking in your conclusion is the word "because," followed by more words. What is the negative impact of chaining for this method? The Experienced Guy wants to know! (Since software engineers are usually required to explain things, rather than cite "feelings.")

@kbrsh
Copy link
Owner

kbrsh commented May 20, 2017

Well, it leads to a smaller bundle size:

// Normal
a.set('count',1);a.set('msg','Hello')

// Chained
a.set('count',1).set('msg','Hello')

// Object
a.set({count:1,msg:'Hello'})

I also feel like the object syntax looks cleaner, instead of chained calls to set. Both have the same performance. Moon debounces calls to set and if you call it more than one time in a block, the app will be built once.

@kbrsh
Copy link
Owner

kbrsh commented Jul 14, 2017

This issue has been inactive for a while, let me know if you are still interested in this being added to Moon.

@kbrsh kbrsh closed this as completed Jul 14, 2017
@jbmonroe
Copy link
Author

I'm good with how you want to do it.

@kbrsh
Copy link
Owner

kbrsh commented Jul 14, 2017

With the object syntax?

@jbmonroe
Copy link
Author

jbmonroe commented Jul 14, 2017 via email

@kbrsh
Copy link
Owner

kbrsh commented Jul 14, 2017

Alright, I can add this as a feature in the next version.

@kbrsh kbrsh reopened this Jul 14, 2017
@oanhnn
Copy link

oanhnn commented Jul 26, 2017

@kingpixil
I have some suggestions:

  • Make moon-loader to working with webpack and integrate to laravel-mix
  • Make github organization and move moon's repositories to organization
  • Make more IDE helper for moon file

@kbrsh
Copy link
Owner

kbrsh commented Jul 26, 2017

@oanhnn Thanks!

  1. I'll make moon-loader, but anyone can easily create a third party tool in the meantime that uses moon-component-compiler.
  2. I'm not so sure about this one, Moon has a small amount of repos that require maintenance (about 3).
  3. moon files already have (really) basic utilities and syntax highlighting if you use Atom. Available as the language-moon package. This can probably be ported easily to other editors as well.

@oanhnn
Copy link

oanhnn commented Jul 26, 2017

I think these three things help expand the moon users community

  1. github organization => moonjs.org
  2. IDE: Atom -> WebStorm, PhpStorm, Eclipse, Netbeans, ...
    And it's community are go big 😄

@kbrsh
Copy link
Owner

kbrsh commented Jul 26, 2017

Thanks for the tips, glad you want to expand the community.

I don't have much time to move all of Moon's repos to an organization right now, or to port syntax highlighting to other editors. (I also don't have a way to test if they work). Porting to sublime should be easy enough, and if you know any people who actively use Moon and would like to take it on, be sure to let them know!

@kbrsh
Copy link
Owner

kbrsh commented Aug 30, 2017

(Finally) fixed in d81083c

@kbrsh kbrsh closed this as completed Aug 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants