WIP: New Plugin Modal

[shn-srl]

Issue Link

DRAFT PR

**Todo: **

• [ ] Fix Tests
• [x] Include all plugins (visibleInSuggestions)
• [ ] Code cleanup
• [ ] Usability Test

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
frontend	✅ Ready (Inspect)	Visit Preview	Aug 19, 2024 8:48am

[github-actions[bot]]

📦 Next.js Bundle Analysis for @serlo/frontend

This analysis was generated by the Next.js Bundle Analysis action. 🤖

🎉 Global Bundle Size Decreased

Page	Size (compressed)
global	104.65 KB (🟢 -54 B)

Details

The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!

Nine Pages Changed Size

The following pages changed size from the code in this PR compared to its base branch:

Page	Size (compressed)	First Load
/___editor_preview	539.81 KB (🟡 +3.7 KB)	644.46 KB
/entity/create/[type]/[taxonomyId]	541.3 KB (🟡 +3.7 KB)	645.96 KB
/entity/repository/add-revision/[...id]	540.54 KB (🟡 +3.7 KB)	645.2 KB
/page/create	540.55 KB (🟡 +3.7 KB)	645.2 KB
/taxonomy/term/create/[parent_id]/[id]	540.28 KB (🟡 +3.7 KB)	644.93 KB
/taxonomy/term/update/[id]	540.2 KB (🟡 +3.7 KB)	644.85 KB
/user/profile/[username]	183.6 KB (🟡 +39 B)	288.25 KB
/user/settings	539.38 KB (🟡 +3.7 KB)	644.04 KB
/willkommen	86.41 KB (🟡 +39 B)	191.06 KB

Details

Only the gzipped size is provided here based on an expert tip.

First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this.

[hejtful]

Nice improvement over the first PR, imo. 👍

A couple of suggestions:

• Rather than rendering the context provider in the InnerDocument, render it in the RowsEditor. Multiple scoped contexts are better for performance, as any changes to it will only trigger re-renders within that one RowsEditor, not the whole editor.
• Consider merging the AllowedChildPlugins context with your new context, as they share the same concern, adding plugins.
• useReducer in your new context might make the code cleaner and easier to understand.

[shn-srl]

Nice improvement over the first PR, imo. 👍

A couple of suggestions:

* Rather than rendering the context provider in the `InnerDocument`, render it in the `RowsEditor.` Multiple scoped contexts are better for performance, as any changes to it will only trigger re-renders within that one `RowsEditor`, not the whole editor.

* Consider merging the `AllowedChildPlugins` context with your new context, as they share the same concern, adding plugins.

* `useReducer` in your new context might make the code cleaner and easier to understand.

Thanks I've implemented what you've suggested, the first two points look good.

I'm not sure if I'm making the cleanest (or most sensible use) of useReducer but in a way I guess it is more readable and stable

[shn-srl]

Sub ticket created

https://linear.app/serlo/issue/PE-69/split-filinthegaps-plugin

[shn-srl]

@elbotho @hejtful

Would appreciate another review round :pray:

I am conducting a full usability test with Gregor today at 16, and still looking over the tests to ensure nothing is failing

[shn-srl]

• there is already a discussion about this but right now I can't replace the text-plugin with the new plugin I select, it always get's appended

Fixed

[hejtful]

Search bar autofocus doesn't work for me in local, on Chrome. Not sure if you're already on it

[shn-srl]

Search bar autofocus doesn't work for me in local, on Chrome. Not sure if you're already on it

It works for me in Chromium and Firefox

Could it be your mouse is over a plugin Item when the modal opens and steals the focus?

[hejtful]

@shn-srl

Could it be your mouse is over a plugin Item when the modal opens and steals the focus?

Yes, but won't that happen most of the time? Can we expect the user's mouse not to be on a plugin item?

[hejtful]

@shn-srl If you change onMouseEnter to onMouseMove in PluginMenuItem, the bug seems to be fixed. Just a suggestion.

[GregorZupan]

Good Idea @hejtful

[shn-srl]

@shn-srl If you change onMouseEnter to onMouseMove in PluginMenuItem, the bug seems to be fixed. Just a suggestion.

Done

[shn-srl]

Updated the description regarding the tests:

Most tests are fixed, flakiness when running locally is making it unsustainable to keep testing locally, as soon as this PR gets merged I will make a follow up PR for any failing tests.

[hejtful]

Most tests are fixed, flakiness when running locally is making it unsustainable to keep testing locally, as soon as this PR gets merged I will make a follow up PR for any failing tests.

Sounds good to me. Just to mention, if you run yarn build then yarn start before running tests, it reduces the flakiness compared to just yarn dev.

[hejtful]

Sometimes, the shortcuts won't work after the modal is opened. They start working after a few clicks on the modal area. Can't find a stable reproduction method, it's on Chrome. Can you reproduce it?

Edit: Fixed by https://github.com/serlo/frontend/pull/4017/commits/96e41ed161a8736352c0d375d074aa66a1f7e99a

[hejtful]

Up and Down arrows don't work in the Text plugin any more. They should move the cursor a line up or down, but instead do nothing.

Edit: Fixed by https://github.com/serlo/frontend/pull/4017/commits/43404ac101fdcebfa341aac10ed34ee187021b65

[shn-srl]

From Botho:

One weird case: When I insert some plugins (e.g. equations & code) with enter I get an additional textplugin below my inserted plugin (I guess the enter propagates)

[hejtful]

@elbotho Fixed the additional Text plugin bug with: https://github.com/serlo/frontend/pull/4017/commits/1d863e745ee569d28878ec5cf0f839954c02f355

Could you please confirm that the removed code is deprecated? I believe it's from when we would add a new Text plugin on Enter key, but we moved away from that feature.

[elbotho]

Could you please confirm that the removed code is deprecated? I believe it's from when we would add a new Text plugin on Enter key, but we moved away from that feature.

I think it is. Thank you.