Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove and replace Frontend Libraries #179

Open
5 of 8 tasks
askvortsov1 opened this issue Oct 2, 2020 · 11 comments
Open
5 of 8 tasks

Remove and replace Frontend Libraries #179

askvortsov1 opened this issue Oct 2, 2020 · 11 comments

Comments

@askvortsov1
Copy link
Member

askvortsov1 commented Oct 2, 2020

Follow-up to flarum/framework#1821

Right now, we're considering:

  • jquery => cash (Typescript, better maintained, smaller) + velocity (separate animation library)
  • Stop using bootstrap js
    • Use a composable component for tooltips (#2843)
      • CSS tooltips instead! (#2697)
  • utils/classList => clsx library (we could export this out under utils/classList like we do with utils/Stream?) (#2760)
  • Replace lodash-es with our own functions (#2827)
    • throttle -> npm throttle-debounce (#2827)
  • Drop spin.js and replace with our own custom loading indicator (#2764)
@askvortsov1 askvortsov1 changed the title Remove and replace jQuery and Bootstrap JS Remove and replace Frontend Libraries Oct 3, 2020
@askvortsov1
Copy link
Member Author

Relevant: flarum/framework#2303 (comment) (replacing bootstrap modals with something better, or a vanilla solution).

@askvortsov1
Copy link
Member Author

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

@askvortsov1
Copy link
Member Author

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

@davwheat
Copy link
Member

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

@askvortsov1
Copy link
Member Author

If it works, I'm all for it!

@askvortsov1
Copy link
Member Author

@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
Copy link
Member

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

@davwheat
Copy link
Member

davwheat commented Mar 14, 2021

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
Copy link
Member

SychO9 commented Mar 14, 2021

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
Copy link
Member

davwheat commented Mar 14, 2021

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
Copy link
Member Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants