vrimar / construct-ui

A Mithril.js UI library
https://vrimar.github.io/construct-ui
MIT License
287 stars 24 forks source link

Tooltip with Button as trigger depresses button on hover #7

Closed angrytongan closed 2 years ago

angrytongan commented 4 years ago

Adding a Button as trigger to a Tooltip renders the button as depressed on hover. Additional button without tooltip renders correctly on hover. Tested in latest Chrome on Mac OS X.

Sample below; not sure if the use of m() as argument for trigger is valid.

'use strict';

const {
    Button,
    FocusManager,
    Tooltip,

} = CUI;
FocusManager.showFocusOnlyOnTab();

const DepressedButtonWithTooltip = () => {
    return {
        view: () => {
            return m('', [
                m(Tooltip, {
                    content: 'Button is depressed while hovering',
                    trigger: m(Button, {
                        label: 'Depressed on hover',
                    }),
                }),
                m(Button, {
                    label: 'Not depressed on hover',
                })
            ]);
        },
    }
};

document.addEventListener('DOMContentLoaded', () => {
    m.route(document.body, '/', {
        '/': DepressedButtonWithTooltip,
    });
});
angrytongan commented 4 years ago

Just checked the Popover sample in the docs and it's the same there. Is this correct behaviour?

vrimar commented 4 years ago

Yeah, that's intended behavior. The cui-active and cui-popover-trigger-active classes are added to the trigger when the tooltip/popover is toggled open.

I could add a triggerActiveClass prop to the existing API to provide better control i.e.

m(Tooltip, {
  content: 'Button is depressed while hovering',
  triggerActiveClass: 'my-class', // or leave it empty to prevent any classes being added when active
  trigger: m(Button, {
    label: 'Depressed on hover',
  }),
}),
angrytongan commented 4 years ago

No worries, no need to do more work. Cheers!

vrimar commented 4 years ago

I think this should stay open, having the button depress on hover doesn't make much sense.