Release version 1.8.1 - Githubissues

dkotter commented 2 months ago

This issue is for tracking changes for the 1.8.1 release. Target release date: August 2024

Release instructions

[x] Branch: Starting from develop, cut a release branch named release/1.8.1 for your changes.
[x] Version bump: Bump the version number in simple-podcasting.php, package-lock.json, package.json, and readme.txt if it does not already reflect the version being released. Update both the plugin "Version:" property and the plugin PODCASTING_VERSION constant in simple-podcasting.php.
[x] Changelog: Add/update the changelog in both CHANGELOG.md and readme.txt.
[x] Props: update CREDITS.md with any new contributors, confirm maintainers are accurate.
[x] New files: Check to be sure any new files/paths that are unnecessary in the production version are included in .distignore.
[x] Readme updates: Make any other readme changes as necessary. README.md is geared toward GitHub and readme.txt contains WordPress.org-specific content. The two are slightly different.
[x] Merge: Make a non-fast-forward merge from your release branch to develop (or merge the pull request), then do the same for develop into trunk, ensuring you pull the most recent changes into develop first (git checkout develop && git pull origin develop && git checkout trunk && git merge --no-ff develop). trunk contains the latest stable release.
[x] Push: Push your trunk branch to GitHub (e.g. git push origin trunk).
[x] Compare trunk to develop to ensure no additional changes were missed.
[ ] Test the pre-release ZIP locally by downloading it from the Build release zip action artifact and installing it locally. Ensure this zip has all the files we expect, that it installs and activates correctly and that all basic functionality is working.
[ ] Release: Create a new release, naming the tag and the release with the new version number, and targeting the trunk branch. Paste the changelog from CHANGELOG.md into the body of the release and include a link to the closed issues on the milestone.
[ ] SVN: Wait for the GitHub Action to finish deploying to the WordPress.org repository. If all goes well, users with SVN commit access for that plugin will receive an emailed diff of changes.
[ ] Check WordPress.org: Ensure that the changes are live on WordPress.org. This may take a few minutes.
[ ] Close the milestone: Edit the milestone with release date (in the Due date (optional) field) and link to GitHub release (in the Description field), then close the milestone.
[ ] Punt incomplete items: If any open issues or PRs which were milestoned for 1.8.1 do not make it into the release, update their milestone to 1.9.0or Future Release.

dkotter commented 2 months ago

In testing the final release zip here, I'm noticing a handful of issues that all seem to stem from the player enhancements we did in #272. I think we need to investigate these a bit more before we proceed here cc / @jeffpaul

Here are issues I'm seeing:

I tested on an existing post that already had a Podcast block in it and I got the dreaded Block recovery error. Luckily clicking that recovery button did work but it would be ideal if this didn't happen. I know we made some big changes to the block but would be great if those changes could be made backwards compatible.
In the settings for each Podcast you can set a Podcast cover image. Within each Podcast block, you can also set an episode image. The preview within the editor always shows the Podcast cover image, not the Podcast block episode image. It does show properly on the front-end though.
We have a separate Podcast Transcript block and if you choose to render the transcript on the front-end, there is no spacing between the Podcast and Transcript (this may be an existing issue, not sure but would be great to address):

I'm getting a PHP warning on the front-end: Attempt to read property "term_id" on string, pointing to includes/blocks/podcast/markup.php:43
I think related to the above, in the Podcast block settings there's a checkbox to set the Podcast. This is in addition to the selector we have at the post level. For existing Podcasts, I'm seeing this box isn't checked at the block level even though it is selected at the post level:

And to continue off the above, not sure why we have this setting duplicated in the block and at the post level. Seems we should standardize on having that in a single location only. I'd prefer to keep it at the post level but if we want it in the block settings, there's some styling changes we should make as you can see from the above screenshot there isn't great spacing between that setting and the one below it

I'm not flagging this as an issue or a blocker for release but I do wonder if we could come up with a better approach for all the new settings we added. Right now it's just a long list of toggles, which works but feels like there may be a better approach:

jeffpaul commented 1 month ago

@dkotter see some response to this in https://github.com/10up/simple-podcasting/pull/318.

iamdharmesh commented 1 month ago

@dkotter Raised https://github.com/10up/simple-podcasting/pull/320 to fix the first issue of list from here: https://github.com/10up/simple-podcasting/issues/313#issuecomment-2304843986

dkotter commented 1 month ago

Thanks to @iamdharmesh and @sudar, most of these have been addressed. Here's the things I still see as outstanding:

[x] Ensure we have proper spacing between the Podcast Transcript block and the Podcast itself

[ ] We resolved an issue with the term ID not being set on the front-end but didn't actually solve why it wasn't being set properly in the admin. Not sure if this is just for existing Podcasts or if new ones have the same problem but if you set the Podcast term at the post level, that isn't being set at the block level:

[ ] Along with the above, do we need this setting twice? Is there any reason this needs to be a block setting instead of a taxonomy set at the post level? Or I guess could also be the opposite, at the block level but not post level. Just doesn't seem to make sense to have it show twice

iamdharmesh commented 1 month ago

@dkotter

Ensure we have proper spacing between the Podcast Transcript block and the Podcast itself

It seems that the podcast transcript shows with proper spacing. Could you please let me know which theme you are using? So, I can check with that.

We resolved an issue with the term ID not being set on the front-end but didn't actually solve why it wasn't being set properly in the admin. Not sure if this is just for existing Podcasts or if new ones have the same problem but if you set the Podcast term at the post level, that isn't being set at the block level:

I am able to reproduce this and raised WIP PR https://github.com/10up/simple-podcasting/pull/322 to fix this

Along with the above, do we need this setting twice? Is there any reason this needs to be a block setting instead of a taxonomy set at the post level? Or I guess could also be the opposite, at the block level but not post level. Just doesn't seem to make sense to have it show twice

I believe this feature was added to improve the user experience (see GitHub issue), but I'm fine with displaying it only once, either at the post level or the block level. Perhaps @jeffpaul might have a better suggestion for this.

Additionally, the behavior of the two panels is currently inconsistent. The post-level panel does not display the hierarchical term selector view, while the block-level panel does. The post-level panel opens the modal for adding a podcast, whereas the block-level panel uses the default core functionality for adding podcasts. We need to match behavior on both or need to display this only once.

Post Sidebar	Block settings

Thank you

dkotter commented 1 month ago

@iamdharmesh

It seems that the podcast transcript shows with proper spacing. Could you please let me know which theme you are using? So, I can check with that

Checked again and not able to reproduce with any of the themes I have installed locally 🤷🏻 So I think we're good on this one

Additionally, the behavior of the two panels is currently inconsistent. The post-level panel does not display the hierarchical term selector view, while the block-level panel does. The post-level panel opens the modal for adding a podcast, whereas the block-level panel uses the default core functionality for adding podcasts. We need to match behavior on both or need to display this only once.

Hmm.. that's interesting, didn't even test all that functionality. I think this was something that just wasn't done entirely correct, especially considering the differences between the two. I really like the modal that shows so my suggestion would be to leave this at the post level and not have it at the block level at all. But I wasn't involved in the decisions around these UI improvements so @jeffpaul can chime in with his thoughts

jeffpaul commented 5 days ago

@10up/open-source-practice with this released blocked, it would be great if someone could help get SVN updated with at least the tested-up-to (currently at 6.6 in develop but could be updated based on #324) as we're otherwise showing 6.4 on dotorg.

https://github.com/10up/simple-podcasting/blob/a237138a99dce200bc0be8e0e3977c5d9c5a5486/readme.txt#L4

10up / simple-podcasting

Release version 1.8.1 #313

Release instructions