Repository navigation
Refactor Complex DOM CSS generator - #246
issackjohn merged 4 commits into
Conversation
Refactored the CSS generator to use JSDOM and html markups.
|
@bgrins can you take a look. @issackjohn refactored the Complex CSS generator to be more flexible. Now we can generate targeted css for the Angular TodoMVC. |
|
Maybe naive question, but why can't we update the Angular markup to match all the others so it doesn't need special handling? |
|
There are some updates in here that look good regardless, but it would be nice if we didn't have to work around a case for Angular specifically. One other question - I see changes between the current CSS and the new CSS after this refactor. For example in the deleted matching.css there were some rules with |
I'm not familiar with angular, we assumed that those custom tags are the usual way to code and went with it. @flashdesignory would it make sense to change those tags to match the others? |
Yes, even though we are using the same seed, the steps of the generator changed a little bit so now it picks different elements. |
I imagine we'll have a similar issue if we introduce something using web components, so we might as well have flexible generator from the beginning. |
what specific markup are we referring to? It's been a while that I looked at this workload... |
The angular version have some extra |
|
Got it... the updated Angular version uses components (todo-list, todo-item, ect), instead of the previous version that was just one file (in speedometer 2.1). That's where these tags are coming from. This is the most common way I saw Angular devs using components in their apps and therefore the most representing way of a typical Angular app. There are probably several different ways to do this differently, but we usually tend to go with the most common approach. |
| @@ -0,0 +1,50 @@ | |||
| export const ANGULAR_TODO_MVC_HTML_MARKUP = ` | |||
| <div class="main-ui" dir="ltr"> | |||
There was a problem hiding this comment.
This doesn't match the markup I see on https://speedometer-preview.netlify.app/resources/todomvc/architecture-examples/angular/dist/.
I was pasting this in the web console to simulate 100 items, although it's not necessary to see the discrepency:
var numberOfItemsToAdd = 100;
var newTodo = document.querySelector(".new-todo");
for (let i = 0; i < numberOfItemsToAdd; i++) {
newTodo.value = `Something to do ${i}`;
newTodo.dispatchEvent(new Event("input"))
enter(newTodo)
}
function enter(node, options = undefined) {
let keyCode = 13;
let eventOptions = { bubbles: true, cancelable: true, keyCode, key: "Enter", which: keyCode };
const event = new KeyboardEvent("keyup", eventOptions);
node.dispatchEvent(event);
}
There was a problem hiding this comment.
We might consider generating the necessary HTML with 100 todo items from the live test contents (starting with something like the above) and pasting it here, compared with needing to keep a duplicate shell and todo generation logic in the script here.
I think that would be an improvement, as it would be easier to keep in sync with changes and I think have less room for error in the codegen script, but I don't feel strongly that it must happen right now.
There was a problem hiding this comment.
These are supposed to match the html structure from here https://issackjohn.github.io/Speedometer/resources/todomvc/architecture-examples/angular/embedded-dist/ , which already contain some of the modifications I introduced in the other PR.. Although, the order of top-bar and ribbon is wrong and must be fixed.
I'd be happy if we could get rid of this manual step down the line.
There was a problem hiding this comment.
Makes sense. I'd like to get #239 landed to make this easier to compare with
It is not only that |
Address PR comments.
@lpardosixtosMs - was this solved? |
@flashdesignory Yes, the latest patch addresses all the comments. |
|
@rniwa - do you want to take a look at this one too, or is it good to merge? |
|
Just resolved a new conflict with .gitignore. |
The html markup must contain a `<app-root>` tag that wraps around the `<section>` tag of the Angular TodoMVC's complex DOM version.
|
Administrative: @issackjohn you should have write access to the repository now. Let me know if you do not. Also, are you on webkit.slack.com? |
@rniwa I still don't have write access. and no, I'm not on there, could you invite me please? |
|
How about now? |
Yeah I have it now. |
In that case, you can go ahead & squash-merge this. |
Refactored the CSS generator to use JSDOM and html markups to create the targeted CSS selectors.
With this PR we can generate CSS for the Complex DOM version of Angular TodoMVC.