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

feat/3448 global font pairs #3449

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Conversation

Arukuen
Copy link
Contributor

@Arukuen Arukuen commented Feb 27, 2025

fixes #3448

Copy link

🤖 Pull request artifacts

file commit
pr3449-stackable-3449-merge.zip e8f2870

github-actions bot added a commit that referenced this pull request Feb 27, 2025
@@ -282,6 +282,53 @@ public function register_global_settings() {
)
);

register_setting(
'stackable_global_settings',
'stackable_font_pair_name',
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest we change this name to something more descriptive to what it is for:

Suggested change
'stackable_font_pair_name',
'stackable_selected_font_pair',


useEffect( () => {
fetchSettings().then( response => {
// Get settings.
setTypographySettings( ( head( response.stackable_global_typography ) ) || {} )
setApplySettingsTo( response.stackable_global_typography_apply_to || 'blocks-stackable-native' )
setCustomFontPairs( response.stackable_custom_font_pairs || [] )
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll need to move this out of free

Comment on lines +275 to +300
const headingStyles = fontPair.value?.find( val => val.selector === 'h1' )?.styles
const paragraphStyles = fontPair.value?.find( val => val.selector === 'p' )?.styles
if ( headingStyles?.fontFamily ) {
loadGoogleFont( headingStyles.fontFamily )
}
if ( paragraphStyles?.fontFamily ) {
loadGoogleFont( paragraphStyles.fontFamily )
}
const label = (
<div>
<span
style={ omit( { ...headingStyles }, [ 'fontSize', 'lineHeight' ] ) }
className="ugb-global-settings-font-pair__label"
>
{ headingStyles?.fontFamily ?? 'Theme Heading Default' }
</span>
<span
style={ omit( { ...paragraphStyles }, [ 'fontSize', 'lineHeight' ] ) }
className="ugb-global-settings-font-pair__sub-label"
>
{ paragraphStyles?.fontFamily ?? 'Theme Body Default' }
</span>
</div>
)

const className = classNames( { 'ugb-global-settings-font-pair__selected': selectedFontPairName === fontPair.name } )
Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend moving this all inside the FontPairPicker component, then adding the necessary props to it:

<FontPairPicker
  // ...
  isSelected={ selectedFontPairName === fontPair.name }
  headingFontFamily={ headingStyles?.fontFamily }
  bodyFontFamily={ paragraphStyles?.fontFamily }
/>

</div>

<div className="ugb-global-settings-font-pair__container" ref={ fontPairContainerRef }>
{ allFontPairs.map( fontPair => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of mapping through allFontPairs and updating the list, you can just output 3 times:

  1. For the default font pair
  2. Custom font pairs - this can be an applyFilters( 'stackable.global-settings.typography.font-pairs.custom', null, selectedFontPairName ) use alternative method below
  3. All other preset font pairs

Copy link
Contributor

Choose a reason for hiding this comment

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

Alternative

// Near the top
const CustomPicker = applyFilters( 'stackable.global-settings.typography.font-pairs.customPicker', Fragment )

// Render the custom font pair pickers
<CustomPicker
  isSelected={ selectedFontPairName === fontPair.name }
  headingFontFamily={ headingStyles?.fontFamily }
  bodyFontFamily={ paragraphStyles?.fontFamily }
/>

In premium:

const CustomFontPairsPicker = props => {
  const [ customFontPairs, setCustomFontPairs ] = setState( [] )

  useEffect( () => {
    // Load all custom font pairs
    const customFontPairs = []
  }, [] )

  return customFontPairs.map( fontPair => {
    return <FontPairPicker
      // ...pass stuff
    />
  }
}

addFilter( 'stackable.global-settings.typography.font-pairs.customPicker', 'stackable/global-settings/font-pairs', CustomFontPairsPicker )

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

Successfully merging this pull request may close these issues.

Global Font Pairs
2 participants