wagtail-nest / wagtail-hallo

Wagtail's legacy Hallo.js richtext editor
https://pypi.org/project/wagtail-hallo/
MIT License
5 stars 7 forks source link

Toolbar not staying within the boundaries of the text area #32

Open cnk opened 1 year ago

cnk commented 1 year ago

I am filing this issue so I have a place to show people what I want to fix. I need some help knowing how to describe this behavior - and where to start looking for how to fix it.

When using the built-in Hallo editor in Wagtail 2.16, the toolbar follows as you scroll up and down a long rich text area.

https://user-images.githubusercontent.com/5952/199617022-77684fc0-92c3-401e-8fd3-e046290134de.mov

But when using wagtail-hallo and Wagtail 3.0.3, the toolbar will happily follow you up the page - rather than coming to rest at the top of the text area when you scroll up and becoming invisible when you scroll down. This behavior starts with the 0.1.0 release of wagtail-hallo.

https://user-images.githubusercontent.com/5952/199617063-79e9d988-d8c6-40dc-a1d4-c1a5ab0a79c1.mov

cnk commented 1 year ago

I have created a Gitpod that demonstrates the floating toolbar problem using bakerydemo. You can spin up your own Gitpod instance using this URL: https://gitpod.io/#https://github.com/cnk/bakerydemo/tree/hallo

Once you have logged in (admin/changeme), edit the home page and note how the toolbar does not stay inside the text area. From the debugging I have managed to do, it appears to me that setPosition is only being called once, when the toolbar gets created the first time: https://github.com/wagtail-nest/wagtail-hallo/blob/main/wagtail_hallo/static/js/vendor/hallo.js#L308 Wagtail uses halloToolbarFixed so setPosition should be called when the window is scrolled but doesn't appear to be doing so. Could it be that these events are not getting attached? https://github.com/wagtail-nest/wagtail-hallo/blob/main/wagtail_hallo/static/js/vendor/hallo.js#L3190

nickmoreton commented 1 year ago

I tried this in Wagtail 4.2 and the issue is still happening. Probably no surprise. 😄

https://user-images.githubusercontent.com/1434115/217304308-b1d6978b-57ce-428b-ac3e-82bf87ebdfb6.mov

coredumperror commented 1 year ago

I found the source of the problem:

hallo.js includes this in the _create() function for halloToolbarFixed:

        jQuery(window).on('resize', function (event) {
          return _this.setPosition();
        });
        jQuery(window).on('scroll', function (event) {
          return _this.setPosition();
        });

Turns out that, for whatever reason, the window no longer scrolls in Wagtail 3 or 4. However, the .content-wrapper element does scroll. So switching out that code for this fixed the toolbar location behavior:

        jQuery('.content-wrapper').on('resize', function (event) {
          return _this.setPosition();
        });
        jQuery('.content-wrapper').on('scroll', function (event) {
          return _this.setPosition();
        });

Still need to figure out how to fix the "resize the input field to make room for the toolbar" behavior, though.

cnk commented 1 year ago

Here is a video of the "resize to make room for the toolbar" behavior from Wagtail 2.16. Since all the javascript in this repo was extracted from Wagtail 2.16, I would guess that the JS is still here but that the HTML has changed so it is no longer finding the same elements it used to use for the calculation.

https://user-images.githubusercontent.com/5952/219878262-e5d52185-2e3e-4512-b615-ce40d42567ce.mov

coredumperror commented 1 year ago

I've put in a PR for this: #39