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

[WIP] V5 #499

Merged
merged 550 commits into from Sep 28, 2019
Merged

[WIP] V5 #499

merged 550 commits into from Sep 28, 2019

Conversation

atomiks
Copy link
Owner

@atomiks atomiks commented May 20, 2019

Tippy v5

The main goal of this version is to:

  1. Improve developer experience with warnings without bloating production bundle size
  2. Massively reduce core size and make the library more treeshake-able
  3. Allow new feature additions to be added separately without increasing bundle size
  4. Improve naming consistency and usage

Highlights

  • ⬇️ 30%+ smaller download size (5.4 kB minzipped including core CSS)
  • ⬇️ 50%+ smaller parse size (15.1 kB minified including core CSS)
  • ⬇️ 60%+ smaller core CSS (0.6 kB minzipped) + smaller external animation files
  • ⚙️ Development and production versions for better developer experience
  • ✨ New animation variations
  • ✨ New touch: ["hold", delay] prop (for long press behavior)
  • ✨ New arrow: string | Element values to use your own shape
  • ✨ New createSingleton method, supersedes group()
  • 🌸 Improved animations integration and documentation, namely fully dynamic transition dimensions when tippy content updates
  • 🌸 Improved naming consistency
  • ♿ Improved accessibility defaults for interactive tippys
  • 🐛 Various minor behavior bug fixes

Development and production versions of Tippy.js

We're now adding helpful development warnings without bloating bundle size. Development files are .js, while production versions are .min.js.

Main filename was renamed from index.all to tippy.bundle for better DevTools display.

tippy.js/
├── dist/
│   ├── tippy.cjs.js              "main" file
│   ├── tippy.esm.js              "module" file
│   ├── tippy-bundle.iife.js              
│   ├── tippy-bundle.iife.min.js         "unpkg" file
│   ├── tippy.iife.js              
│   ├── tippy.iife.min.js
// This whole block will get stripped by minifiers/DCE for production builds
// When importing like `import tippy from 'tippy.js'`
if (process.env.NODE_ENV !== 'production') {
  if (somethingBad) {
    console.warn('You did somethingBad');
  }
}

Where's the umd folder? The browser format is now iife. Node is covered by cjs, and amd is dead and/or didn't work with popper.js/tippy.js anyway before because of the .js suffix apparently (which is what umd covered). Now it's not necessary.

iife development version includes the block without the Node-only process variable. In the .min version, it's stripped out entirely (like the other formats).


Drop Android 4.4 support

Remove all -webkit- prefixed transforms to save 200 minzipped bytes or so.


Force data-placement and data-out-of-boundaries attributes

The inconsistency between data-animation and x-placement has been annoying. Since 4.2.0, we've moved these attributes to the .tippy-tooltip node, so we can rename them.


Default is now arrow: true and animation: 'fade'

Having animateFill: true and animation: 'shift-away' animation as defaults add a lot of CSS bloat. By making the "bootstrap" tooltip default a lot of code is stripped away.

Further reasons listed here: #499 (comment)

import tippy from 'tippy.js';
import 'tippy.js/dist/backdrop.css';
import 'tippy.js/animations/shift-away.css';

tippy(targets, {
  content: 'tooltip',
  animateFill: true,
  animation: 'shift-away'
});

If using a custom SVG arrow, import the SVG arrow CSS

Again, for treeshaking reasons, not everyone cares about SVG arrows, so will need to import the CSS separately:

import 'tippy.js/dist/svg-arrow.css';

Delays are always 0 in touch and keyboard contexts

I don't see a reason to keep delay here, it's poor UX. If people require this to be configured then we can add it later.

Remove target option and tippy.group() from core

They've been moved into a file called "addons" to conserve core's size. createSingleton has replaced group since its behavior is better and allows transitions since a single tippy is being used.

Functions in this bundled file should be treeshaken by bundlers.

import {delegate, createSingleton} from 'tippy.js';

delegate('#parent', {
  target: 'button'
});

const singleton = createSingleton(tippy('button'), {
  delay: 500
});

Move followCursor to plugins

Like the "addons", props are being moved to separate parts because not everyone needs this behavior.

import tippy, {followCursor} from 'tippy.js';

tippy.use(followCursor);

Animations

A ton of work has been gone into considering animations more deeply.

This includes:

  • CSS transitions (default)
  • CSS animations (e.g. animate.css)
  • JavaScript and spring animations (e.g. animejs)
  • FLIP animations of tippy content (e.g. react-flip-toolkit/core)

Default CSS animations: shift-toward, scale, perspective animations removed

Only fade will remain in tippy.css.

Extra CSS animations will be moved to a folder called animations/ (like themes/); there are now 3 variants of each animation expect for fade (subtle and extreme added).

Example import:

import 'tippy.js/animations/shift-away-subtle.css';

CSS animations (instead of transitions - e.g. animate.css)

We're now using use top/left/right/bottom: 10px as the base distance offset, rather than specifying it in the transform. Makes it more compatible with external animation CSS, FLIP libraries, and removes need for the notransition technique.

There's no need to think about distance anymore now due to this.

tippy('button', {
  // `fade` is now just an opacity transition, which is a good base for most
  // animations.
  animation: 'fade',
  onMount(instance) {
    const {tooltip} = instance.popperChildren;
    requestAnimationFrame(() => {
      tooltip.classList.add('animated', 'wobble');
    });
  },
  onHidden(instance) {
    const {tooltip} = instance.popperChildren;
    tooltip.classList.remove('animated', 'wobble');
  }
});

Dimensions transition

⚠️ Highly experimental code. There are tons of edge cases I've had to work out. Hopefully this can be wrapped in a simple addon API.

gif

See demo/flip

Remaining problems:

  • Work out the dynamic clip-path based on the placement needed to prevent content overflowing when the tippy has an arrow (since it can't have overflow: hidden).
  • Work out how to make it fully dynamic, considering .setProps() method.

Themes

  • google theme was renamed to material, to remove the association with Google.
  • Prevent the need to specify .tippy-tooltip[data-animatefill] .tippy-backdrop selector (to make background-color transparent)

New lifecycle functions

  • onCreate
  • onUntrigger
  • onPropsUpdated
  • onDestroy

Virtual reference elements

Plain objects are not supported anymore - instead, you just pass a placeholder element like this:

const placeholderElement = document.createElement('div');

// IE9+ supports overriding this method. That's all that's needed for
// Popper.js to position the tippy.
//
// `client{Width,Height}` when 0 (i.e. not appended to the document) do not have an effect,
// as Popper.js falls back to the dimensions instead
placeholderElement.getBoundingClientRect = () => ({ ... });

tippy(placeholderElement, options);

This conserves size since we don't need to polyfill element methods in the plain object.

Auto built index.d.ts

Internal types don't live in types.ts so now index.d.ts gets generated on build, which is just a copy of src/types.ts.

Naming

The term "options" is being completely ditched in favor of "props" everywhere. And now consistent methods:

  • instance.set() was renamed to instance.setProps()
  • tippy.defaults was renamed to tippy.defaultProps
  • tippy.setDefaults() was renamed to tippy.setDefaultProps()

This directly aligns with the React component model since every prop is dynamic and updateable. Options are now Partial, or termed "optional props". The merged interface is Props as it was before.

  • showOnInit -> showOnCreate to match lifecycle hook

New static property

tippy.currentInput is a mutable object with a single property isTouch. This moves the internal isUsingTouch flag to this object so that users can access it, as it's sometimes useful in rare cases.

i.e. tippy.currentInput.isTouch

New accessibility defaults for interactive tippys

By default if interactive: true, then: {aria: null, appendTo: "parent"}

There's also a warning in DEV if the tippy is not directly after the ref/triggerTarget node. If the user specifies a different appendTo, then it gets ignored as it assumes there are clipping issues and they need a custom focus management solution to handle it.

  • aria-expanded attribute also added
  • aria-describedby handles multiple tippys

TODO

5.1.0 goals:

  • Wrap animejs logic in an addon wrapper
  • Wrap FLIP animation library logic in an addon wrapper, hopefully solve the 2 remaining problems

Current experimental React wrapper

@KubaJastrz KubaJastrz self-requested a review May 20, 2019 07:25
Copy link
Collaborator

@KubaJastrz KubaJastrz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's so much stuff to go through, although I haven't found anything standing out at first glance. Great job 💪

src/addons/transitionDimensions.ts Outdated Show resolved Hide resolved
src/utils.ts Outdated Show resolved Hide resolved
@atomiks
Copy link
Owner Author

atomiks commented May 23, 2019

I released 5.0.0-alpha.0 (@next tag). Anyone want to play with it?

@atomiks
Copy link
Owner Author

atomiks commented May 23, 2019

"main" field file doesn't exist woops 👀 but the "module" does so it works with bundlers anyway for now if you want to give it a whirl!

All the imports and types files seem to work properly (esp. with the addons folder) which is good news.

addons/index.d.ts Outdated Show resolved Hide resolved
@atomiks
Copy link
Owner Author

atomiks commented May 24, 2019

One last thing about the API that has annoyed me is the whole props/options/config etc terminology.

Current and inconsistent

tippy.defaults;
tippy.setDefaults(Options);
instance.props;
instance.set(Options);

Consistent, but Options represents “optional” right? Not the merged interface with the defaults… that was my original problem with this. The options they pass in aren't the same as instance.options

tippy.defaultOptions;
tippy.setDefaultOptions(Options);
instance.setOptions(Options);
instance.options;

Consistent, dropping “Options” completely in favour of props, never mentioning Options.

tippy.defaultProps;
tippy.setDefaultProps(Partial<Props>);
instance.setProps(Partial<Props>);
instance.props;

Need your help with this @KubaJastrz 🤣

@KubaJastrz
Copy link
Collaborator

KubaJastrz commented May 24, 2019

I'm not sure where options term came from. It might refer to, as you said, optional params of a function. In our case this is something more, users can dynamically change those parameters.

I like the terminology of component-based pattern, where you have instances of a component and different properties you can use them with (props).

I was really confused when we were making the first typings earlier 😛 It's good to make it more consistent finally.

Edit: I'm not sure if this made any sense, but props > options IMO 😅

@atomiks
Copy link
Owner Author

atomiks commented May 24, 2019

In our case this is something more, users can dynamically change those parameters.

I like the terminology of component-based pattern, where you have instances of a component and different properties you can use them with (props).

Really nice explanation. I think you're right here!

@atomiks
Copy link
Owner Author

atomiks commented May 31, 2019

I think we're getting close here... I'll release another alpha soon

@atomiks
Copy link
Owner Author

atomiks commented Jun 1, 2019

Released 5.0.0-alpha.1

Remaining things to do:

  • Extensive manual visual testing in all rendering engines especially IE11 which can be very weird (mainly with the new animations etc.)
  • Ensure treeshaking works properly in webpack
  • ?? @KubaJastrz

I'm planning a June 8-9 final release, next weekend

This should be the last major in a while, definitely no more this year anyway (unless Popper.js 2 arrives soon). The next one will be with Popper.js 2 if it ever arrives, or possibly a custom positioning engine or fork of Popper.js... who knows if that will happen in the future 😅

@KubaJastrz
Copy link
Collaborator

Why am I on your todo list 😂
I'll make sure to review it, don't worry

@atomiks
Copy link
Owner Author

atomiks commented Jun 2, 2019

From what I could test, IE11 renders things fine visually. Same with the other rendering engines, Safari, Firefox, iOS Safari.

IE11 did bug on the FLIP demo sometimes, but that's related to my current buggy WIP implementation of it and not this actual V5 core code.

@atomiks
Copy link
Owner Author

atomiks commented Jun 2, 2019

I'm still not sure about the themes change:

v4

import 'tippy.js/themes/light.css';

tippy('button', {
  theme: 'light'
});

Current PR

import 'tippy.js/themes/light.css';

tippy('button', {
  theme: 'tippy-light'
});

Consistent

Need to consider the __NAMESPACE_PREFIX__ thing here for the filename:

Themes use classList instead of [data-theme] because you can have 2 or more themes together, unlike animations.

import 'tippy.js/themes/tippy-light.css';

tippy('button', {
  theme: 'tippy-light'
});

Just revert to v4's lack of prefix?

Haven't heard of CSS style inheriting complaints with themes at all (mainly just global styles).

@KubaJastrz
Copy link
Collaborator

Need to consider the __NAMESPACE_PREFIX__ thing here for the filename

This whole __NAMESPACE_PREFIX__ is kinda confusing, since you are injecting it into all warnings and error messages apart from class names and themes.
On the other hand, it would be better to rename filenames too 🤷‍♂️

Just revert to v4's lack of prefix?

Haven't heard of CSS style inheriting complaints with themes at all (mainly just global styles).

Poluting the global styles is not a very good idea despite lack of complaints 😛
But you are right, apart from #484, there weren't any requests for that.

@mreinstein, what your thoughs on that?

@atomiks
Copy link
Owner Author

atomiks commented Jun 4, 2019

Poluting the global styles is not a very good idea despite lack of complaints 😛

Our CSS isn't polluting because it's prefixed with the .tippy-tooltip selector, but the tippy element can inherit CSS on the document within a .dark-theme selector rule.

Maybe a good compromise:

Use v4's lack of prefix, but the default theme should be '' (empty string) and get ignored and never added. That way by default, it won't inherit any styles due to the unprefixed theme class. The included themes can cause problems still, but they aren't default and the user can adjust any conflicting CSS if they want to use them.

@mreinstein
Copy link
Contributor

mreinstein commented Jun 4, 2019

@mreinstein, what your thoughs on that?

Thanks for reaching out!

To be honest I'm not sure I understand what is being suggested in the above PR comment. I can tell you what my use case is: I'm including tippy as a 3rd party module in my code bundle that gets embedded on some very large retail websites, and they have particular demands about not polluting their web page globals with styles and/or javascript.

Here is what my build process looks like when I'm including tippy in distributed code:

git clone git@github.com:atomiks/tippyjs.git
cd tippyjs
npm install
NAMESPACE=acmecorp npm run build
cp umd/index.all.min.js ~/Sites/acmecorp/scripts/lib/tippy/index.all.min.js

Then I copy the contents of tippyjs/themes/light-border.css into a css variable in
acmecorp/scripts/lib/tippy/tippy.js

This means that all tippy related css starts with acmecorp-, including the light-border theme related css. Because this stuff is namespaced the big clients we have don't freak out when we embed our code/styles on their web pages, because they know acmecorp- is our style namespace and it won't conflict.
Anyway sorry for the length of this post, just wanted to illuminate the use case behind #484 in more detail. I'm really hoping this doesn't break with v5 🙏

@atomiks
Copy link
Owner Author

atomiks commented Jun 5, 2019

So just to make things clear:

The CSS is like this for every selector:

.tippy-iOS {}
.tippy-popper {}
.tippy-tooltip.light-theme {}

I consider it non-polluting because it has the library's name as a prefix, which is its own namespace unless the website is using tippy for something else. In cases where you're using tippy deliberately I don't see the problem.

Where I did see a problem is if the user had .light-theme {} as its own selector in their CSS, then the tippy element will be affected, not the other way around where tippy's CSS affects their own CSS.

And also, I've always relied on attributes like [data-state] to be unlike class names in terms of inheriting CSS problems, as I rarely think they are "bare" like this:

[data-state='visible'] {
  /* breaking tippy here */
}

Which is why I didn't use data-tippy-state -- it's only used for attributes on reference elements because tippy shouldn't affect their own elements, but it can do whatever it wants with the tippy elements itself.


Regarding the "3rd party module" thing:

It would be nice if you could change the prefix at runtime instead of build-time. The CSS files are the main problem.

For the main CSS - only thing I can think of is doing something to the bundled file to make all of the tippy parts a variable that can be set via a static method, and the injecting is deferred with setTimeout like auto-initialization.

const css = "...";
let namespacePrefix = 'tippy';

function injectCSS(css) {/*...*/}

setTimeout(() => {
  injectCSS(css.replace(/tippy/g, namespacePrefix);
});

// ...
tippy.setNamespacePrefix = str => {
  namespacePrefix = str;
});

tippy.css + the themes however can't be changed, so it does seem to be a build-time only thing.

@mreinstein
Copy link
Contributor

It would be nice if you could change the prefix at runtime instead of build-time.
The CSS files are the main problem.

I actually think it's superior having it done at build time. I have no need to dynamically change this, and having less javascript and other crap around at run time is all the better.

Many of the websites that host these 3rd party widgets have piles upon piles of 3rd party junk running. Anything I can do to make it more static and cut down on the amount of unnecessary dynamism helps make the experience better.

@mreinstein
Copy link
Contributor

one thing that would be super nice (and if this is difficult it's not that big of a deal) but I'd love to be able to have another environment variable I could pass in at the build step, which changes the default bundled theme. e.g., if I wanted to bundle material with my tippy build:

NAMESPACE=acmecorp THEME=material npm run build

I know we talked about this in #484 and it was not trivial, but maybe now that you're already in there doing a major version bump this isn't that crazy of a change? :-)

@atomiks
Copy link
Owner Author

atomiks commented Jun 5, 2019

I don't think that is too hard to do

My guess is something like this

function createPluginSCSS(output) {
  const theme = process.env.THEME
  const data =
    `$namespace-prefix: ${NAMESPACE_PREFIX};` +
    (theme ? `@import './themes/${theme}.scss';` : '')

  return sass({
    output,
    options: { data },
    processor(css) {
      return postcss([autoprefixer, cssnano])
        .process(css, { from: undefined })
        .then(result => result.css)
    },
  })
}

@atomiks
Copy link
Owner Author

atomiks commented Jun 5, 2019

So it was really easy 😆

@atomiks
Copy link
Owner Author

atomiks commented Jun 8, 2019

@KubaJastrz
I think it's ready for review, when you get time 🏂

@atomiks
Copy link
Owner Author

atomiks commented Jun 13, 2019

Thinking of replacing interactiveBorder with an SVG like this 👀

https://twitter.com/i/status/1132932935399673857

@KubaJastrz
Copy link
Collaborator

Sorry for the delays but I'm having a pretty busy week. I even missed your ping 😅
I'll have a look at it tomorrow, any particular things I should focus on?

@atomiks
Copy link
Owner Author

atomiks commented Jun 14, 2019

Sorry for the delays but I'm having a pretty busy week. I even missed your ping 😅

It's okay there's no rush anyway

I'll have a look at it tomorrow, any particular things I should focus on?

Just glaring flaws/mistakes that seeped through. It might be difficult to understrand the nuances of various things without rigorously playing around with it yourself

Full list of development demos (I'm going to add a ui folder for the demo):

npm run dev # Single reference element button, use to develop new features
npm run dev:themes # Develop new themes, ensure existing themes are working
npm run dev:animations # Develop new animations, ensure existing animations are working
npm run dev:flip # The holy grail of dimensions transitions
npm run dev:addons # delegate + createSingleton, for now
npm run dev:ui # Common UI patterns: nested hover menus, tooltip+popover on single element combo, combobox, dropdowns, etc.

I did attempt to make followCursor an addon but it hooks deeply into parts of the instance lifecycle, especially the private sections, and didn't work too great. I think it stays in core...

Copy link
Collaborator

@KubaJastrz KubaJastrz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Code looks much simpler now, especially rollup config.

I've noticed that there are copied tippy bundles in the website directory, maybe you could use bundles from root dir?

tsconfig.json Show resolved Hide resolved
rollup.pre-build.js Outdated Show resolved Hide resolved
src/types.ts Outdated Show resolved Hide resolved
@atomiks
Copy link
Owner Author

atomiks commented Jun 16, 2019

I've noticed that there are copied tippy bundles in the website directory, maybe you could use bundles from root dir?

I think my concern with that was you need to not have the directory cleaned. But npm i also runs npm prepare which builds the files so ... 🤔

@KubaJastrz
Copy link
Collaborator

Well, maybe we could copy the bundles into website dir but have it gitignored? And just add command to rebuild tippy to website readme

@atomiks
Copy link
Owner Author

atomiks commented Sep 28, 2019

Let's finally merge & release.. 😳

@atomiks atomiks merged commit edd9d4f into master Sep 28, 2019
@atomiks atomiks deleted the v5 branch September 28, 2019 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants