Add molecules logo. - Githubissues

moxie-lean / ng-patternlab

Atomic Design Patternlab in Angular for use with the LEAN Ecosystem

http://getmoxied.net/lean

MIT License

0 stars 1 forks source link

Add molecules logo. #70

Closed nacho87 closed 7 years ago

nacho87 commented 7 years ago

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Add new molecule to patterns

What is the current behavior? (You can also link to an open issue here)
What is the new behavior (if this is a feature change)?

Can use molecule logo

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
Other information:

mitogh commented 7 years ago

Please fill this information.

https://github.com/moxie-lean/ng-patternlab/pull/70#issue-158267701

adamf321 commented 7 years ago

Do we need a component / directive for this? Can't it just be defined with HTML / CSS, like a <h1>?

adamf321 commented 7 years ago

Also I think this is an atom, not a molecule

andybar2 commented 7 years ago

@adamf321

Atoms should be simple html tags. This is a composition of atoms because it has a link and an image. That's why we used a molecule...

mitogh commented 7 years ago

It looks good to me @andybar2 let me know if you have any comments for the @adamf321 questions / concerns otherwise we can move forward.

adamf321 commented 7 years ago

Yes, but it's just one visual element on the page. I guess it comes down to how we define an "atom".

To my other point - could we just give the example of how to put in on the page using html / css only? I'm thinking that for simple elements (all atoms and some molecules) we could do this to cut down the amount of work the browser has to do when rendering.

mitogh commented 7 years ago

What do you mean by:

we could do this to cut down the amount of work the browser has to do when rendering.

On Jun 3, 2016, at 9:40 AM, Adam Fenton notifications@github.com wrote:

To my other point - could we just give the example of how to put in on the page using html / css only? I'm thinking that for simple elements (all atoms and some molecules) we could do this to cut down the amount of work the browser has to do when rendering.

adamf321 commented 7 years ago

If you use <ln-m-logo href="..." ...></ln-m-logo> the browser has to run JS on this to convert it into the template before rendering it (sorry, "rendering" perhaps wasn't the right word).

However if you write <a href="..." class="ln-m-logo"><img src="..."></a> the browser skips the JS step, so it's more efficient.

andybar2 commented 7 years ago

I really don't think that this introduces a significant overhead @adamf321 , and mostly considering that we use directives for everything... Having it as a component gives us the possibility to change in only one place if we want to add something to the template in the future (i.e. a tooltip).

mitogh commented 7 years ago

Yes I think so this is just a minor optimisation that is not really required at this point.

adamf321 commented 7 years ago

In this case, sure. But we could have loads of them. But fine, we can leave it.

I still think this is an atom though... not a molecule.