flarum / issue-archive

0 stars 0 forks source link

Remove and replace Frontend Libraries #179

Open askvortsov1 opened 3 years ago

askvortsov1 commented 3 years ago

Follow-up to https://github.com/flarum/core/issues/1821

Right now, we're considering:

askvortsov1 commented 3 years ago

Relevant: https://github.com/flarum/core/issues/2303#issuecomment-751206229 (replacing bootstrap modals with something better, or a vanilla solution).

askvortsov1 commented 3 years ago

Relevant: #2303 (comment) (replacing bootstrap modals with something better, or a vanilla solution would fix that issue).

askvortsov1 commented 3 years ago

Should also strive to use vanilla JS wherever possible to make the eventual transition simpler.

davwheat commented 3 years ago

Have you considered using clsx as opposed to classnames? It's a drop-in replacement and far, far smaller.

askvortsov1 commented 3 years ago

If it works, I'm all for it!

askvortsov1 commented 3 years ago

@datitisev I think we should also include your idea of tooltips as a composable component as part of this. Do we have a separate issue for that?

dsevillamartin commented 3 years ago

@askvortsov1 I don't think we have a separate issue; I don't remember making one & wasn't able to find one.

davwheat commented 3 years ago

I'm going to try and push for modals to use the native <dialog> element as opposed to any custom JS-based solutions.

It's supported in Firefox and Chrome for Android, and Google even made their own polyfill which works with their desktop version of Chrome.

Check out the demos to see what's possible: https://googlechrome.github.io/dialog-polyfill/


We can also use CSS-only tooltips, like these: https://kazzkiq.github.io/balloon.css/

Using JS tooltips where not needed isn't a good idea as DOM manipulation takes far more time for devices to handle and paint than just a CSS transform. It also means that we don't need to worry about "creating" the tooltips.

My API suggestion for this would be something like this:

<button aria-label="This will delete your forum" data-tooltip>
  My cool button
</button>

Using CSS's attr(), we can use a pseudoelement to show a tooltip:

// Show tooltip only if we have both attributes
[data-tooltip][aria-label] {
  &::after {
    content: attr(aria-label);
  }
}

It's a good idea to use aria-label for this, as it will force people to use a11y-friendly attributes to get the result they want. This is a pretty common practice in newer CSS frameworks, and you can read more about it here: https://tech.ebayinc.com/engineering/how-our-css-framework-helps-enforce-accessibility/

SychO9 commented 3 years ago

I love that CSS is powerful enough to do things like tooltips, but the problem with this solution is that it uses psuedo elements to achieve that, quoting:

Drawbacks Balloon.css make use of pseudo-elements thus self-closing elements such as <img>, <input> and <hr> will not render tooltips.

Also keep in mind that if pseudo elements are already in use on an element, the tooltip will conflict with them resulting in potential bugs.

davwheat commented 3 years ago

Any issue arising from the use of pseudoelements could be resolved immediately by wrapping the node in a plain div or span.

If we wanted, core could include a tooltip helper component to do this without anyone thinking about it.

<Tooltip text="Hello">
  <img />
</Tooltip>
askvortsov1 commented 3 years ago

@datitisev I think we should also include your idea of tooltips as a composable component as part of this. Do we have a separate issue for that?

A tool tip helper component has been suggested and I think it's a great idea to promote consistency in UI and abstract away the implementation.