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

Workshops: Move layout elements from post_content to theme. #203

Merged
merged 3 commits into from
Mar 31, 2021

Conversation

iandunn
Copy link
Member

@iandunn iandunn commented Mar 24, 2021

Having them in post_content means that we have to manually update all existing posts every time we change the layout.

See #195, https://wordpress.slack.com/archives/C01KC5VDQBC/p1616436392000400

Closes #201

Blocker for #200

@iandunn iandunn added [Type] Enhancement New feature request for the Learn website. [Component] Learn Theme Website development issues related to the Learn theme. [Component] Learn Plugin Website development issues related to the Learn plugin. [Component] Content Website development issues related to the content on Learn. labels Mar 24, 2021
Having them in `post_content` means that we have to manually update all existing posts every time we change the layout.
@iandunn iandunn force-pushed the workshop-template-reduction branch from 253f8f4 to b1e8102 Compare March 24, 2021 22:05
@iandunn iandunn marked this pull request as ready for review March 24, 2021 22:14
Copy link
Contributor

@coreymckrill coreymckrill left a comment

Choose a reason for hiding this comment

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

👍 Looks good, and doesn't seem to break anything. A few comments below.

Also, I wonder if while we're at this, we should just get rid of the 'template_lock' => 'all' property of the workshop CPT entirely. I think we set it originally to make sure that the stuff in the sidebar didn't get accidentally deleted, and now that the sidebar is hard-coded, it seems like more of a hinderance than anything else.

wp-content/plugins/wporg-learn/inc/post-meta.php Outdated Show resolved Hide resolved
Comment on lines +17 to +22
<textarea
id="workshop-video-url"
name="video-url"
class="large-text"
rows="4"
><?php echo esc_url( $post->video_url ); ?></textarea>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why a textarea instead of just a text input?

Copy link
Member Author

Choose a reason for hiding this comment

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

There's never enough room to display the full URL in a text input, even with the .large-text class

@iandunn
Copy link
Member Author

iandunn commented Mar 26, 2021

Removed template_lock in 4a6a082

@iandunn iandunn merged commit 9005221 into trunk Mar 31, 2021
@iandunn iandunn deleted the workshop-template-reduction branch March 31, 2021 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Component] Content Website development issues related to the content on Learn. [Component] Learn Plugin Website development issues related to the Learn plugin. [Component] Learn Theme Website development issues related to the Learn theme. [Type] Enhancement New feature request for the Learn website.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants