jquery / jquery-ui

The official jQuery user interface library.
https://jqueryui.com
Other
11.26k stars 5.32k forks source link

Drop animation is broken #1991

Open BSarmady opened 3 years ago

BSarmady commented 3 years ago

jQuery: v3.6.0 jQuery UI: - v1.12.1 (with 3.6.0) - 2021-06-12 jQuery UI: - v1.13.0 (with 3.6.0) - 2021-06-12 (still broken)

Drop animation is broken, I'm not sure about other animations since I haven't tried. Basically, after completing the animation hide-drop animation, the element is showing again. however this is happening in some specific case. This animation works correctly in jQuery UI - v1.11.4 - 2021-06-28

I tried using jQuery 3.6.0 but unfortunately lots of other UI elements, animation and components are broken too so I had to downgrade back to jQuery 1.11.3 to get website working again.

Issue Clicking on menu1\~3 will hide title and then show submenu1\~3. After mouse leave the menu, animation will trigger (a 1 second timer) and hide submenu1~3 and show title again. This doesn't work on menu1 and 2 but works on menu3. after completing hide-drop animation, sub menu 1 and 2 show again over title which is also visible.

Additional Info Now if you add

<div id="menu4" class="submenu hidden"></div>

after menu 3, menu 3 will show same symptoms too but not menu4. Same way if you remove submenu 3, sub menu 2 will work correctly (last menu item is the only one working as expected).

This works correctly on 1.11.4 but something else is broken in 1.11.4 and that was my reason to upgrade to 1.12.1

Following is the test code that I am using (attached as file too)

<!DOCTYPE html>
<html>
<head>
    <meta charset="utf-8" />
    <meta http-equiv="content-type" content="text/html; charset=utf-8">
    <meta content="width=device-width, initial-scale=1, maximum-scale=1, user-scalable=no" name="viewport" />
    <script type="text/javascript" src="https://code.jquery.com/jquery-3.6.0.min.js"></script>
    <!--script type="text/javascript" src="https://code.jquery.com/jquery-1.11.3.min.js"></script-->
    <!--script type="text/javascript" src="https://code.jquery.com/ui/1.11.4/jquery-ui.min.js"></script-->
    <script type="text/javascript" src="https://code.jquery.com/ui/1.12.1/jquery-ui.min.js"></script>
    <script type="text/javascript">
        var MenuTimer;
        $(function () {
            $('.button').button();
            $('.menu a').hover(
                function () {
                    clearTimeout(MenuTimer);
                },
                function () {
                    MenuTimer = setTimeout(function () {
                        $('.submenu').hide('drop', { direction: 'up' });
                        $('.title').show('drop', { direction: 'right' });
                        $('.menu a').blur();
                    }, 1000);
                }
            )

            $('.menu').on('click', 'a', function () {
                $('.submenu').hide();
                $('.title').hide('drop', { direction: 'right' });
                $('#' + $(this).attr('data-id')).show('drop', { direction: 'up' });
                if ($(this).attr('href') == '#')
                    return false;
            });
        });
    </script>
    <style type="text/css">
        .hidden { display: none; }
        .title { position: absolute; top: 0px; left: 0px;background-color:red }
        .submenu { position: absolute; top: 0px; left: 0px;background-color:green }
        .menu { flex-grow: 2; display: flex; flex-direction: column; }
        .header { height: 126px; }
    </style>
</head>
<body>
    <div class="header">
        <div class="m-1 menu">
            <div class="mainmenu">
                <a href="#" class="button" data-id="menu1">menu1</a>
                <a href="#" class="button" data-id="menu2">menu2</a>
                <a href="#" class="button" data-id="menu3">menu3</a>
            </div>
            <div style="flex-grow:2; position:relative;">
                <div class="title">TITLE TITLE TITLE TITLE TITLE</div>
                <div id="menu1" class="submenu hidden">
                    <a href="#" class="button">Sub Menu 1</a>
                </div>
                <div id="menu2" class="submenu hidden">
                    <a href="#" class="button">Sub Menu 2</a>
                </div>
                <div id="menu3" class="submenu hidden">
                    <a href="#" class="button">Sub Menu 3</a>
                </div>
            </div>
        </div>
    </div>
</body>
</html>

1.html.txt

mgol commented 3 years ago

Thanks for the report. Can you provide a test case on JS Bin? Thanks!

Just to make it clear: does the animation work correctly with jQuery 1.11.3 and is only broken with 3.6.0? In which browsers?

mgol commented 3 years ago

I think the issue is you're triggering a drop hide animation on all submenus when you hover out of any of the menu entries. If you trigger the animation only on the proper one, the animation works fine.

mgol commented 3 years ago

See an example without the bug (code to find the proper element is quite hacky, I didn't focus on that part): https://jsbin.com/deholic/1/edit?html,css,js,output

BSarmady commented 3 years ago

I made even a smaller case (drop the whole thing in jsbin html tab and it will work)

<!DOCTYPE html>
<html>
<head>
    <meta charset="utf-8" />
    <meta http-equiv="content-type" content="text/html; charset=utf-8">
    <meta content="width=device-width, initial-scale=1, maximum-scale=1, user-scalable=no" name="viewport" />
    <script type="text/javascript" src="https://code.jquery.com/jquery-3.6.0.min.js"></script>
    <!--script type="text/javascript" src="https://code.jquery.com/ui/1.11.4/jquery-ui.min.js"></script-->
    <script type="text/javascript" src="https://code.jquery.com/ui/1.12.1/jquery-ui.min.js"></script>
    <script type="text/javascript">
        var MenuTimer;
        $(function () {
          setTimeout(function () {
            $('.submenu').hide('drop', { direction: 'up' });
            $('.title').show('drop', { direction: 'right' });
            $('.menu a').blur();
          }, 1000);
        });
    </script>
    <style type="text/css">
        .hidden { display: none; }
        .title { position: absolute; top: 0px; left: 0px;background-color:red }
        .submenu { position: absolute; top: 0px; left: 0px;background-color:green }
        .menu { flex-grow: 2; display: flex; flex-direction: column; }
        .header { height: 126px; }
    </style>
</head>
<body>
    <div class="header">
        <div class="m-1 menu">
            <div style="flex-grow:2; position:relative;">
                <div class="title hidden">TITLE TITLE TITLE TITLE TITLE</div>
                <div id="menu1" class="submenu">
                    <a href="#" class="button">Sub Menu 1</a>
                </div>
                <div id="menu2" class="submenu hidden">
                    <a href="#" class="button">Sub Menu 2</a>
                </div>
            </div>
        </div>
    </div>
</body>
</html>

I'm guessing it has something to do with final step of animation which should apply to all elements (0\~n) in array but only applies to elements 1\~n.

mgol commented 3 years ago

From my investigation, the issue is the mode method used to set the effect mode sets it to "none" when mode is "hide" but the element is already hidden. If you try to hide all the submenus with the animation, mode is called multiple times during the hiding process and in cases other than the first it gets set to "none" as the element is already marked as hidden.

That's all I can do for now; given the maintenance status of jQuery UI and the fact this is not a regression in 1.13 but older, it's unlikely to get fixed unless someone from the community submits a PR.

BSarmady commented 3 years ago

I would gladly use UI 1.13 with 3.6.0 if it worked, but this issue exist in 1.13 too.

Looking at repo changes, mentioned method was added in commit (b6bec79) Effects: Rewrite

changes were made in a commit at 2012-12-26 (before release of 1.11) but merged in after 2 years and before release of 1.12.0

Unfortunately the mentioned commit is very extensive and removing second if in method doesn't seem to be a viable scenario as it looks like lots of other methods rely on the way this method works now.

mgol commented 3 years ago

I would gladly use UI 1.13 with 3.6.0 if it worked, but this issue exist in 1.13 too.

I understand that. But we're not focusing on bugs that were introduced before 1.13, there are too few people involved in the project. It's been many years since that release so people already had to adapt to this issue in their projects.

If you want it fixed, your best chance is to try to fix it yourself and submit a PR. Otherwise, it'll likely stay unfixed.