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

Resizable Editor Feedback/Ideas #21468

Open
mikeselander opened this issue Apr 7, 2020 · 1 comment
Open

Resizable Editor Feedback/Ideas #21468

mikeselander opened this issue Apr 7, 2020 · 1 comment
Labels
[Type] Discussion For issues that are high-level and not yet ready to implement.

Comments

@mikeselander
Copy link
Contributor

Hi folks, I've been working on integrating a feature similar to the Resizable Editor experiment into a client's site and I have some feedback on this implementation. I used the core implementation as a guideline and found several bugs and room for improvement in this feature:

You can also see this feedback inline to the code here: #19082 (review)

  1. As far as I can tell, we're pulling stylesheets anew every single time this runs which is unnecessary. I think this could be wrapped in useMemo to prevent it from constantly re-loading.
  2. The logic for limiting the styles is moderately useful for Gutenberg-written stylesheets but untenable for theme and plugin stylesheets. As an example, I'm currently working on porting a client over to Gutenberg and they have 200+ individual SCSS files that are used in the theme. Adding this rule that makes no sense in any other context to a stylesheet is polluting and doesn't make sense for the other 10 teams working on the site. I would propose that a better rule would be to look for any rules with .editor-styles-wrapper or .wp-block classes in the cssText. This would allow us to not pollute stylesheets with invalid and nonsense rules.
  3. Could we have a filter to be able to set the default view that authors see when loading the page? Several clients I know of would like to have a mobile-first experience, while other would like desktop view to be the default.
  4. Could have a filter or setting to be able to modify the breakpoint values? They will change per theme and per client as different designs require different breakpoints.
  5. I have the following concerns about the stylesheet selection logic:
    1. I believe this excludes any styles passed in via add_editor_style as they're added in an inline style element.
    2. We should additionally be excluding any print stylesheets as they're not useful to this logic.
    3. We should also be excluding most plugin stylesheets from this list as they don't have any block-list related styles. This probably seems like a micro-optimization, but adding just ACF & Yoast can add 20-30 stylesheets to the editor view which is a lot more rules to be parsing. This would be really difficult to do on a granular basis, but I believe my next point would be able to help with this.
    4. To reduce the number of rules to parse when triggering a change, we could check for valid rules in a stylesheet and only push through those stylesheets which have parsable rules. On the site I'm integrating this into this change + culling out plugins stylesheets cut the stylesheets being passed here from 60+ to 15. This dramatically sped up the rule parsing when triggering the viewport change.
@talldan talldan added the [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues. label Apr 20, 2020
@talldan
Copy link
Contributor

talldan commented Apr 20, 2020

cc'ing @tellthemachines who worked on this feature originally.

@youknowriad youknowriad added [Type] Discussion For issues that are high-level and not yet ready to implement. and removed [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues. labels Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Discussion For issues that are high-level and not yet ready to implement.
Projects
None yet
Development

No branches or pull requests

3 participants