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

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

Open
cnk opened this issue Nov 2, 2022 · 5 comments
Open

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

cnk opened this issue Nov 2, 2022 · 5 comments
Labels
help wanted Extra attention is needed

Comments

@cnk
Copy link
Collaborator

cnk commented Nov 2, 2022

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.

Wagtail2.16BuiltinHallo.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.

Wagtail3.0WagtailHallo.mov
@cnk
Copy link
Collaborator Author

cnk commented Feb 6, 2023

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

nickmoreton commented Feb 7, 2023

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

Screen.Recording.2023-02-07.at.16.24.37.mov

@cnk cnk mentioned this issue Feb 7, 2023
@cnk cnk added the help wanted Extra attention is needed label Feb 7, 2023
@coredumperror
Copy link

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

cnk commented Feb 18, 2023

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.

Screen.Recording.2023-02-18.at.8.55.29.AM.mov

@coredumperror
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants