SMH110 / widgets

1 stars 0 forks source link

HTML is getting difficult to maintain #41

Closed massimocode closed 8 years ago

massimocode commented 8 years ago

I'm looking at the widget branch which will add the todo list and I think the HTML is getting difficult to maintain. It's not nice to have the HTML for the calculator and the todo list in one place.

I think we should look at using Knockout components. This way, the HTML for the calculator and todo list can be separate from the main page and even loaded dynamically using Require JS.

SMH110 commented 8 years ago

I have tried to do that but it didn't work https://github.com/SMH110/My-Calculator/commit/af350dcd40ac752066e529fb59155243221f81be

massimocode commented 8 years ago

I think we should look at using Knockout components.

http://knockoutjs.com/documentation/component-overview.html

HTML binding is something different and not what we would want to use. Having said that, as much as I really dislike the approach, you could probably get it working by doing this: html: calculatorHtml.calcHtmlCode but it's really not pretty at all.

You would also have to edit calculator.js to use ES6 backticks to wrap the HTML as you are using a multi line string i.e. ` instead of '

You could use single quotes as you are doing now but you would have to escape the end of every line with a backslash.

SMH110 commented 8 years ago

This html: calculatorHtml.calcHtmlCode didn't work https://github.com/SMH110/widgets-/commit/5f996d83e8f752338353851f77057bc03eead608

massimocode commented 8 years ago

That's correct, because the HTML has been rendered but data-bind attributes within the HTML were ignored.

We should be looking at using knockout components instead. I'll see if I can put in a PR for it.

massimocode commented 8 years ago

I've created a PR for this here https://github.com/SMH110/widgets/pull/48. Please checkout the updated readme.

massimocode commented 8 years ago

The PR converts the calculator to a component. Feel free to go ahead and checkout the branch and convert the todo list to a component as well.

SMH110 commented 8 years ago

Done here 43e800d