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

Control preview automatically #128

Open
wants to merge 1 commit into
base: docs/extend
Choose a base branch
from

Conversation

ashfame
Copy link
Member

@ashfame ashfame commented Nov 25, 2024

This PR attempts to explore the possibility of being able to control the preview automatically without having to do it separately.

I have added the filter call within the function responsible for storing reference, so that developers just have to do a single call and everything is taken care of.

This exposes a flaw in our current approach (#125) of letting multiple plugins compete for a subject type. In reality, we want the user to evaluate multiple options to see which one they want. Eventually, only one would/should win and be the final transformed output of that subject.

Additionally, only one preview can be rendered at any time, so it doesn't make sense to have multiple plugins compete & transform at the same time.

Its like buying shoes really, you want to evaluate multiple ones to see which one you like, even try them one by one but you can't try multiple ones at the same time and eventually you only buy the best one.

This is why in my previous approach (#123) I made it a point to detect when plugins compete and only advance with just the chosen plugin with previews functional without having to do anything. So, I propose the solution of being able to resolve this by exposing a dropdown in the UI for instance.

With this PR, I hope to finalize the decision of how we should handle previews.

@ashfame ashfame self-assigned this Nov 25, 2024
@ashfame ashfame requested review from akirk and psrpinto November 25, 2024 22:14
add_filter(
'data_liberation_preview_transformed_post_id',
function () use ( $transformed_post_id ) {
return $transformed_post_id;
Copy link
Member

Choose a reason for hiding this comment

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

I'm having some trouble understanding the purpose of this filter. What would be a use case where something other than $transformed_post_id would be returned?

Copy link
Member Author

Choose a reason for hiding this comment

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

$transformed_post_id is the native transformed post id with filter on top of it, so plugins can modify its value to have their transformed post id be returned and hence used for preview.

Copy link
Member Author

Choose a reason for hiding this comment

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

The result is obviously ambiguous when multiple plugins are competing since only value can be returned and shown in preview. The winning one would be the one that hooks last & would overwrite the value.

Copy link
Member

Choose a reason for hiding this comment

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

Could you give an example of a use case where the plugin would need to do both the following:

  1. Call store_reference() with a $transformed_post_id with value A
  2. Return a different value than A in the data_liberation_preview_transformed_post_id filter

In what situations do you see this happening?

Copy link
Member Author

Choose a reason for hiding this comment

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

Literally every case :) Lets say X plugin takes over S subject type and run its transformation logic to produce P post_id post. We have also produced N post_id post with our native transformation.

With current approach, P doesn't automatically gets used for preview. So, this filter comes into play, where the plugin author would first plug in their own transformation and then return P as the post_id using the filter so that gets used for preview. Hope that makes it clear?

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for clarifying. If I'm understanding this correctly, that means that the decision of which transformed post (N or P) is used for the preview becomes the responsibility of plugins?

If so, what advantages does that bring compared to it being the responsibility of "core try-wordpress" to decide what gets used as the preview?

In other words, why should it be a plugin that decides what transformed post gets used for preview? To me this feels like a core responsibility of try-wordpress, I'm not sure I see the advantages of exposing that concept to plugins.

Base automatically changed from feedback_abstract_relationship_setting to docs/extend December 4, 2024 14:28
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.

2 participants