Skip to content
This repository has been archived by the owner on Oct 16, 2024. It is now read-only.

Don't handle the theme anymore, only the theme's patterns #3

Merged
merged 39 commits into from
Jan 17, 2023

Conversation

kienstra
Copy link
Contributor

@kienstra kienstra commented Jan 11, 2023

What does this do?

  • Removes the theme from the data model. Instead, it uses patterns (previously .included_patterns).
  • Removes wp-modules/string-fixer/ and other modules that were FSE-specific.
  • Changes the get-theme endpoint to get-patterns, and their associated functions, like get_theme()
  • Refactors useThemeData() to be for patterns, not themes.

What doesn't this do

  • Change markup
  • Implement Mike's really nice designs

Testing steps

  1. Might have to do rm -rf pattern-manager && git clone -b update/get-theme https://github.com/studiopress/pattern-manager
  2. Of course, if you do that, push all of your local commits first
  3. nvm use 14 && npm i && npm run dev
  4. Create a new pattern
  5. Add some content
  6. Click 'Save' (might have to do this twice)
  7. Reload the page
  8. Expected: your new pattern appears with the edits you made

Base automatically changed from update/simplify-pm-app to main January 11, 2023 22:21
template_part = 'template_parts',
}

export type PatternType = keyof typeof ThemePatternType;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is such a nice one, it's hard to delete 😄

'type' => 'object',
'description' => __( 'The patterns', 'pattern-manager' ),
'validate_callback' => function( $to_validate ) {
return is_array( $to_validate );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would have been nice, but it didn't work:

'validate_callback' => 'is_array',

@@ -0,0 +1,140 @@
<?php
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed from api-theme-data.php, as this is for patterns, not themes.

return array(
'patterns' => \PatternManager\PatternDataHandlers\get_patterns(),
'theme' => \PatternManager\ThemeDataHandlers\get_theme( $current_theme_data['id'] ?? null ),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need the whole theme anymore, just the patterns.

data is not an optional property,
so we shouldn't need the ? operator.
} );
}

function savePatternsData() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed from saveThemeData()

return new Promise( ( resolve ) => {
setIsSaving( true );

fetch( patternmanager.apiEndpoints.savePatternsEndpoint, {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO for another PR: rename patternmanager to patternManager

template_parts?: string[];
tested_up_to: string;
text_domain: string;
theme_json_file?: { [ key: string ]: unknown };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Michael, sorry to delete the nice typing you did.

// Try to prevent populating an empty title by only updating if the type is a pattern.
// Doing it this way should still catch an empty title if the user somehow passes one.
if ( postMeta?.type === 'pattern' ) {
if ( postMeta.title ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no postMeta.type anymore, as this editor only handles patterns, not templates.

{ postMeta?.type === 'template' ? (
<TemplateDetails postMeta={ postMeta } />
) : (
<>
Copy link
Contributor Author

@kienstra kienstra Jan 13, 2023

Choose a reason for hiding this comment

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

This ternary isn't needed, as there's no postMeta.type anymore.

I just removed the ternary, without changing anything other than formatting.

@kienstra kienstra marked this pull request as ready for review January 13, 2023 23:56
@kienstra
Copy link
Contributor Author

kienstra commented Jan 13, 2023

You were probably hoping for a PR before the 3-day weekend 🤣

Of course, no rush.

@kienstra kienstra changed the title Don't handle handle the theme anymore, only the theme's patterns Don't handle the theme anymore, only the theme's patterns Jan 14, 2023
@@ -8,12 +8,11 @@ import React from 'react';
import { patternmanager } from '../../globals';

import PatternManagerContext from '../../contexts/PatternManagerContext';
import PatternManagerSnackbarContext from '../../contexts/PatternManagerNoticeContext';
import NoticeContext from '../../contexts/NoticeContext';
Copy link
Contributor Author

@kienstra kienstra Jan 17, 2023

Choose a reason for hiding this comment

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

Just a renaming. The new name is more generic.

@@ -42,15 +42,12 @@ export default function TitlePanel( {
);
}, [] );

/**
/*
Copy link
Contributor Author

@kienstra kienstra Jan 17, 2023

Choose a reason for hiding this comment

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

Nitpick: this isn't a JSDoc, just a comment.

@@ -3,43 +3,20 @@ import { PostMeta } from '../types';
export default function useChangeWords( postMeta: PostMeta ) {
// Change the word "Publish" to "Save Pattern"
function changeWords( translation: string, text: string ) {
if ( postMeta?.type === 'pattern' ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no postMeta.type anymore, as we're not editing templates. Only patterns.

@mike-day mike-day mentioned this pull request Jan 17, 2023
6 tasks
} from '../types';
import { ThemePatternType } from '../enums';

export default function useThemeData(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mainly moved to usePatterns(), and removed theme-only logic.

@@ -15,7 +15,7 @@ export default function PatternPreview( { url, scale }: Props ) {
const [ iframeRef, setIframeRef ] = useState<
HTMLIFrameElement | undefined
>( undefined );
patterns?.addRef( url, iframeRef );
patterns.addRef( url, iframeRef );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A nice thing about your TS typing is that we know which properties are guaranteed to be there.

Copy link
Contributor

@mike-day mike-day left a comment

Choose a reason for hiding this comment

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

Excellent work, @kienstra!

This is a really nice cleanup. The model focuses specifically on patterns now, and it's much easier to find what's needed without the extraneous data.

Also, you resolved conflicts with the UI rough-in super fast!

@mike-day
Copy link
Contributor

Nice!

Screen Shot 2023-01-17 at 1 02 00 PM

@kienstra
Copy link
Contributor Author

Hi @mike-day,

This is a really nice cleanup. The model focuses specifically on patterns now, and it's much easier to find what's needed without the extraneous data.

Thanks so much for your kind words and review!

@kienstra kienstra merged commit 9a91222 into main Jan 17, 2023
@kienstra kienstra deleted the update/get-theme branch January 17, 2023 19:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants