Skip to content

Remove the name and element props from the TypographyPanel component #47908

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.

Sign up for GitHub

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

Merged
youknowriad merged 4 commits into from
Feb 15, 2023
Merged

Remove the name and element props from the TypographyPanel component #47908

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0783fd0
Remove the name and element props from the TypographyPanel component
youknowriad Feb 9, 2023
732e14e
Fix typo
youknowriad Feb 9, 2023
15317d7
Fix typography panels in global styles
youknowriad Feb 14, 2023
21802dc
Simplification
youknowriad Feb 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Remove the name and element props from the TypographyPanel component
  • Loading branch information
@youknowriad
youknowriad committed Feb 14, 2023
commit 0783fd0467ac57833e47dd7fd8a7507a6acc38f6
72 changes: 72 additions & 0 deletions packages/block-editor/src/components/global-styles/hooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,3 +218,75 @@ export function useSupportedStyles( name, element ) {

return supportedPanels;
}

/**
* Given a settings object and a list of supported panels,
* returns a new settings object with the unsupported panels removed.
*
* @param {Object} settings Settings object.
* @param {string[]} supports Supported style panels.
*
* @return {Object} Merge of settings and supports.
*/
export function overrideSettingsWithSupports( settings, supports ) {
Copy link
Contributor Author

@youknowriad youknowriad Feb 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is that ultimately this function is going to allow us to merge "block.json" supports with "theme.json" settings "like" object.

const updatedSettings = { ...settings };

if ( ! supports.includes( 'fontSize' ) ) {
updatedSettings.typography = {
...updatedSettings.typography,
fontSizes: {},
customFontSize: false,
};
}

if ( ! supports.includes( 'fontFamily' ) ) {
updatedSettings.typography = {
...updatedSettings.typography,
fontFamilies: {},
};
}

if ( ! supports.includes( 'lineHeight' ) ) {
Copy link
Member

@jorgefilipecosta jorgefilipecosta Feb 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: All the rules from now on could be replaced with a single forEach loop:

[ 'lineHeight', 'fontStyle', ....].forEach(...)

updatedSettings.typography = {
...updatedSettings.typography,
lineHeight: false,
};
}

if ( ! supports.includes( 'fontStyle' ) ) {
updatedSettings.typography = {
...updatedSettings.typography,
fontStyle: false,
};
}

if ( ! supports.includes( 'fontWeight' ) ) {
updatedSettings.typography = {
...updatedSettings.typography,
fontWeight: false,
};
}

if ( ! supports.includes( 'letterSpacing' ) ) {
updatedSettings.typography = {
...updatedSettings.typography,
letterSpacing: false,
};
}

if ( ! supports.includes( 'textTransform' ) ) {
updatedSettings.typography = {
...updatedSettings.typography,
textTransform: false,
};
}

if ( ! supports.includes( 'textDecoration' ) ) {
updatedSettings.typography = {
...updatedSettings.typography,
textDecoration: false,
};
}

return updatedSettings;
}
1 change: 1 addition & 0 deletions packages/block-editor/src/components/global-styles/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export {
useGlobalStylesReset,
useGlobalSetting,
useGlobalStyle,
overrideSettingsWithSupports,
} from './hooks';
export { useGlobalStylesOutput } from './use-global-styles-output';
export { GlobalStylesContext } from './context';
Expand Down
135 changes: 34 additions & 101 deletions packages/block-editor/src/components/global-styles/typography-panel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,29 +17,16 @@ import LineHeightControl from '../line-height-control';
import LetterSpacingControl from '../letter-spacing-control';
import TextTransformControl from '../text-transform-control';
import TextDecorationControl from '../text-decoration-control';
import { useSupportedStyles } from './hooks';
import { getValueFromVariable } from './utils';

export function useHasTypographyPanel( name, element, settings ) {
const hasFontFamily = useHasFontFamilyControl( name, element, settings );
const hasLineHeight = useHasLineHeightControl( name, element, settings );
const hasFontAppearance = useHasAppearanceControl(
name,
element,
settings
);
const hasLetterSpacing = useHasLetterSpacingControl(
name,
element,
settings
);
const hasTextTransform = useHasTextTransformControl(
name,
element,
settings
);
const hasTextDecoration = useHasTextDecorationControl( name, element );
const hasFontSize = useHasFontSizeControl( name, element, settings );
export function useHasTypographyPanel( settings ) {
const hasFontFamily = useHasFontFamilyControl( settings );
const hasLineHeight = useHasLineHeightControl( settings );
const hasFontAppearance = useHasAppearanceControl( settings );
const hasLetterSpacing = useHasLetterSpacingControl( settings );
const hasTextTransform = useHasTextTransformControl( settings );
const hasTextDecoration = useHasTextDecorationControl( settings );
const hasFontSize = useHasFontSizeControl( settings );

return (
hasFontFamily ||
Expand All @@ -52,52 +39,38 @@ export function useHasTypographyPanel( name, element, settings ) {
);
}

function useHasFontSizeControl( name, element, settings ) {
const supports = useSupportedStyles( name, element );
Copy link
Contributor Author

@youknowriad youknowriad Feb 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now this function is only called once for the "TypographyPanel" rendering, there might be a small perf improvement here (very hard to measure). I believe this addresses one of the points raised by @jorgefilipecosta in the original PR.

function useHasFontSizeControl( settings ) {
const disableCustomFontSizes = ! settings?.typography?.customFontSize;
const fontSizesPerOrigin = settings?.typography?.fontSizes ?? {};
const fontSizes =
fontSizesPerOrigin?.custom ??
fontSizesPerOrigin?.theme ??
fontSizesPerOrigin.default;
return (
supports.includes( 'fontSize' ) &&
( !! fontSizes?.length || ! disableCustomFontSizes )
);
return !! fontSizes?.length || ! disableCustomFontSizes;
}

function useHasFontFamilyControl( name, element, settings ) {
const supports = useSupportedStyles( name, element );
function useHasFontFamilyControl( settings ) {
const fontFamiliesPerOrigin = settings?.typography?.fontFamilies;
const fontFamilies =
fontFamiliesPerOrigin?.custom ??
fontFamiliesPerOrigin?.theme ??
fontFamiliesPerOrigin?.default;
return supports.includes( 'fontFamily' ) && !! fontFamilies?.length;
return !! fontFamilies?.length;
}

function useHasLineHeightControl( name, element, settings ) {
const supports = useSupportedStyles( name, element );
return (
settings?.typography?.lineHeight && supports.includes( 'lineHeight' )
);
function useHasLineHeightControl( settings ) {
return settings?.typography?.lineHeight;
}

function useHasAppearanceControl( name, element, settings ) {
const supports = useSupportedStyles( name, element );
const hasFontStyles =
settings?.typography?.fontStyle && supports.includes( 'fontStyle' );
const hasFontWeights =
settings?.typography?.fontWeight && supports.includes( 'fontWeight' );
function useHasAppearanceControl( settings ) {
const hasFontStyles = settings?.typography?.fontStyle;
const hasFontWeights = settings?.typography?.fontWeight;
return hasFontStyles || hasFontWeights;
}

function useAppearanceControlLabel( name, element, settings ) {
const supports = useSupportedStyles( name, element );
const hasFontStyles =
settings?.typography?.fontStyle && supports.includes( 'fontStyle' );
const hasFontWeights =
settings?.typography?.fontWeight && supports.includes( 'fontWeight' );
function useAppearanceControlLabel( settings ) {
const hasFontStyles = settings?.typography?.fontStyle;
const hasFontWeights = settings?.typography?.fontWeight;
if ( ! hasFontStyles ) {
return __( 'Font weight' );
}
Expand All @@ -107,27 +80,16 @@ function useAppearanceControlLabel( name, element, settings ) {
return __( 'Appearance' );
}

function useHasLetterSpacingControl( name, element, settings ) {
const setting = settings?.typography?.letterSpacing;
const supports = useSupportedStyles( name, element );
if ( ! setting ) {
return false;
}
return supports.includes( 'letterSpacing' );
function useHasLetterSpacingControl( settings ) {
return settings?.typography?.letterSpacing;
}

function useHasTextTransformControl( name, element, settings ) {
const setting = settings?.typography?.textTransform;
const supports = useSupportedStyles( name, element );
if ( ! setting ) {
return false;
}
return supports.includes( 'textTransform' );
function useHasTextTransformControl( settings ) {
return settings?.typography?.textTransform;
}

function useHasTextDecorationControl( name, element ) {
const supports = useSupportedStyles( name, element );
return supports.includes( 'textDecoration' );
Copy link
Contributor Author

@youknowriad youknowriad Feb 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice that the previous implementation of text transform didn't take into consideration the "setting" value that comes from theme.json. I think this is an inconsistency and probably something that was "forgotten". So in that sense, the current PR changes the behavior a bit but I believe it's better anyway.

function useHasTextDecorationControl( settings ) {
return settings?.typography?.textDecoration;
}

function TypographyToolsPanel( { ...props } ) {
Expand All @@ -146,8 +108,6 @@ const DEFAULT_CONTROLS = {

export default function TypographyPanel( {
as: Wrapper = TypographyToolsPanel,
name,
element,
value,
onChange,
inheritedValue = value,
Expand All @@ -159,11 +119,7 @@ export default function TypographyPanel( {
getValueFromVariable( { settings }, '', rawValue );

// Font Family
const hasFontFamilyEnabled = useHasFontFamilyControl(
name,
element,
settings
);
const hasFontFamilyEnabled = useHasFontFamilyControl( settings );
const fontFamiliesPerOrigin = settings?.typography?.fontFamilies;
const fontFamilies =
fontFamiliesPerOrigin?.custom ??
Expand All @@ -188,7 +144,7 @@ export default function TypographyPanel( {
const resetFontFamily = () => setFontFamily( undefined );

// Font Size
const hasFontSizeEnabled = useHasFontSizeControl( name, element, settings );
const hasFontSizeEnabled = useHasFontSizeControl( settings );
const disableCustomFontSizes = ! settings?.typography?.customFontSize;
const fontSizesPerOrigin = settings?.typography?.fontSizes ?? {};
const fontSizes =
Expand All @@ -213,16 +169,8 @@ export default function TypographyPanel( {
const resetFontSize = () => setFontSize( undefined );

// Appearance
const hasAppearanceControl = useHasAppearanceControl(
name,
element,
settings
);
const appearanceControlLabel = useAppearanceControlLabel(
name,
element,
settings
);
const hasAppearanceControl = useHasAppearanceControl( settings );
const appearanceControlLabel = useAppearanceControlLabel( settings );
const hasFontStyles = settings?.typography?.fontStyle;
const hasFontWeights = settings?.typography?.fontWeight;
const fontStyle = decodeValue( inheritedValue?.typography?.fontStyle );
Expand All @@ -247,11 +195,7 @@ export default function TypographyPanel( {
};

// Line Height
const hasLineHeightEnabled = useHasLineHeightControl(
name,
element,
settings
);
const hasLineHeightEnabled = useHasLineHeightControl( settings );
const lineHeight = decodeValue( inheritedValue?.typography?.lineHeight );
const setLineHeight = ( newValue ) => {
onChange( {
Expand All @@ -266,11 +210,7 @@ export default function TypographyPanel( {
const resetLineHeight = () => setLineHeight( undefined );

// Letter Spacing
const hasLetterSpacingControl = useHasLetterSpacingControl(
name,
element,
settings
);
const hasLetterSpacingControl = useHasLetterSpacingControl( settings );
const letterSpacing = decodeValue(
inheritedValue?.typography?.letterSpacing
);
Expand All @@ -287,11 +227,7 @@ export default function TypographyPanel( {
const resetLetterSpacing = () => setLetterSpacing( undefined );

// Text Transform
const hasTextTransformControl = useHasTextTransformControl(
name,
element,
settings
);
const hasTextTransformControl = useHasTextTransformControl( settings );
const textTransform = decodeValue(
inheritedValue?.typography?.textTransform
);
Expand All @@ -308,10 +244,7 @@ export default function TypographyPanel( {
const resetTextTransform = () => setTextTransform( undefined );

// Text Decoration
const hasTextDecorationControl = useHasTextDecorationControl(
name,
element
);
const hasTextDecorationControl = useHasTextDecorationControl( settings );
const textDecoration = decodeValue(
inheritedValue?.typography?.textDecoration
);
Expand Down
Loading