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

Fixed Random Collapse of Colors Setting Section #32388

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

@thisissandip
Copy link
Contributor

@thisissandip thisissandip commented Jun 2, 2021

Fixes #32295

Description

Fixed Random Collapse of Colors Setting Section on various selection.

The collapse used to occur when the link color is:

  1. selected for the first time
  2. removed/cleared and text/Background color settings are selected right after that.

This was happening because the withElementsStyles HOC returned different components when the link color is set and when it's not. Instead, withElementsStyles HOC should return a single component with different props based on the link color using conditional rendering.

How has this been tested?

  1. Select Block which supports color settings (eg. paragraph block)
  2. Open sidebar settings.
  3. Open Colors and select a custom color from one of the options.
  4. The color settings panel should not collapse like it used to before.

Screenshots

Color.Collapse.Fix.Test.mov

Types of changes

Bug fix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

``

@@ -246,9 +246,7 @@ export const withBlockControls = createHigherOrderComponent(
const withElementsStyles = createHigherOrderComponent(
( BlockListBlock ) => ( props ) => {
const elements = props.attributes.style?.elements;
if ( ! elements ) {
return <BlockListBlock { ...props } />;

This comment has been minimized.

@youknowriad

youknowriad Jun 2, 2021
Contributor

Yes, this causes a remount which is not great for BlockEdit components. We had similar issues in other hooks in the past. So yeah the fix here is looking good to me.

} }
/>
{ elements && (
<style

This comment has been minimized.

@youknowriad

youknowriad Jun 2, 2021
Contributor

Aside: (something for other PRs) Also noting that inline styles like that (which we do in layout as well), can cause CSS issues due to the new elements breaking :first-child selectors and things like that in editor styles.

@jorgefilipecosta @nosolosw @ellatrix We may need a dedicated way to inject these styles in the editor (and maybe even in the frontend) that doesn't suffer from these issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants