aeternity / aepp-components

deprecated: aepp-components to be used in all aepps.
ISC License
41 stars 14 forks source link

Update PR: "Add AeListItem from the Base app" #213

Closed sadiqevani closed 5 years ago

sadiqevani commented 5 years ago

The code in the pull-request https://github.com/aeternity/aepp-components/pull/152 for the component ae-list-item is too specific and too narrow for its general purpose.

I don't want to couple the new components tightly with other old components. So I've improved the code a bit to make the list item more of a general scope and not as strict as it was in the previous pull request.

sadiqevani commented 5 years ago

@davidyuk

I tried the component and works good with the to and tag attributes. Now what I can think of to improve this component, would be to apply router-link when to attribute gets applied on the component, but this will increase complexity.

davidyuk commented 5 years ago

Now what I can think of to improve this component, would be to apply router-link when to attribute gets applied on the component

It was already implemented, you shouldn't have removed this stuff. to prop without router-link is useless.

@edwarddikgale What happening here?

sadiqevani commented 5 years ago

@davidyuk

davidyuk commented 5 years ago

Why AeLink is going to be deprecated? Because of this component in CamelCase? AeLink was a solution for the same question about dependency on RouterLink what was raised by Philipp a long time ago.

sadiqevani commented 5 years ago

Why AeLink is going to be deprecated? Because of this component in CamelCase? AeLink was a solution for the same question about dependency on RouterLink what was raised by Phillip a long time ago.

It's a useless component. And I'm assuming that the problem was raised by Philipp when router-link was used internally in components.

The components are not going to use internal router-link (the new ones), if there will be need to replace the component tag with a router-link, then we'll use a different approach.

davidyuk commented 5 years ago

Actually, I forgot about it, can be really better to use AeLink instead of RouterLink, great that you have noticed it!

Providing a tag prop to render the right element is the way to go, rather than making conditions on what element to use

This property makes this component too flexible, in my version I have defined a set of options that are actually used.

Let's add AeComponent as a wrapper around Component and rewrite all projects using it.

davidyuk commented 5 years ago

AeLink was implemented to add to property of AeButton component without explicit dependency on vue-router. It was very convenient to use it, I don't know why you break it.

davidyuk commented 5 years ago

Implement a mechanism that replaces the component tag with router-link as was done in ae-list-item

That if RouterLink component of the different routing package for vue (have you seen them somewhere?) will accept route in a different property than to? This approach won't work.

davidyuk commented 5 years ago

So, to make a button as a hyperlink I should always wrap it with my own RouterLink adding useless dom nodes and breaking semantic, thank you Sadi!

@emil-apeunit @edwarddikgale @dadaphl you are talking that this guy should make decisions regarding the components package?

sadiqevani commented 5 years ago

@davidyuk

  1. Leave your temper and irony outside when you're talking, they're not welcomed.
  2. router-link has the following prop, if you feel inclined in wrapping router-link on top of a button: https://router.vuejs.org/api/#tag
  3. If you're facing an issue, just write up an issue ticket on the components repository, your behaviour is not constructive, and does not fix anything.
  4. HTML5 Standard says that block level elements can go inside <a></a>
  5. If you're the guy who fights over "semantics", then you should also agree that based on accessibility, a LINK should never look/feel like a BUTTON!
    1. https://marcysutton.com/links-vs-buttons-in-modern-web-applications/
    2. http://www.webaxe.org/proper-use-buttons-links/
    3. https://www.sitepoint.com/community/t/anchors-without-href/6690