search

onOffice-Web-Org / oo-wp-plugin

onOffice for WP-Websites
https://wp-plugin.onoffice.com
GNU General Public License v3.0
9 stars 9 forks source link

P#65189 Non-detail shortcodes in ACF are recognized as detail page #225

Closed jmaas-onoffice closed 2 years ago

jmaas-onoffice commented 2 years ago

Current state

After I add a custom field as described in #220, I can correctly set the detail page using the shortcode in ACF instead of Gutenberg. grafik

I then add a new page and set a shortcode for an estate list instead of the detail page shortcode. grafik

After saving, this page is now considered the new detail page: grafik

The cause is probably that the code doesn't check the view parameter of the shortcode: https://github.com/onOfficeGmbH/oo-wp-plugin/pull/184/files#diff-36b0349987efd5fb1177605d12886ce9d5a493e28ed7459aa63a721d7bc9991fR48

Desired state

The plugin should only set the detail page if the shortocde is actually the detail view shortcode.

tang-hien-egs commented 2 years ago

@jmaas-onoffice , i want to clear a few issue :

  1. when user create a new page detail, should we prioritize check shortcode in page content of page old or check shortcode in custom field in page new ?
  2. i find issue : when remove custom field groups contains shortcode detail, plugin not set page detail. This is issue I fixed, is it requirement ?
  3. i find issue : when delete custom field in custom field group contains shortcode detail, plugin not set page detail. This is issue I fixed, is it requirement ?
jmaas-onoffice commented 2 years ago

@tang-hien-egs I'm not sure that I understand some of the points.

  1. I'm not sure what you mean with old and new page. Can you give an example of what you think should happen?
  2. What do you mean by "remove custom field groups"? I'm also not sure what the fixed behavior is: Does it set the detail page or not? Maybe an example could help me.
  3. Similar to 2., what is the new behavior?
tang-hien-egs commented 2 years ago

example 1: the first,i create a custom field as issue #220, then i add shortcode in page content at page "page_detail". issue1_65189 After, i create page "issue_225" and add shortcode in custom field at page "issue_225 issue_225 " i want clean : now the plugin will set page detail is "page_detail"(the page has shortcode at page content) or "issue_225"(the page has shortcode at custom field).

jmaas-onoffice commented 2 years ago

Oh, I think I understand now, thanks for the example!

It does not matter if the shortcode is in the page or in a custom field. It should detect both types and the detail page should be set to the one that was saved last.

So in your example, it would set the detail page to issue_225 because you saved that page last.

Does this help?

tang-hien-egs commented 2 years ago

Oh, I think I understand now, thanks for the example!

It does not matter if the shortcode is in the page or in a custom field. It should detect both types and the detail page should be set to the one that was saved last.

So in your example, it would set the detail page to issue_225 because you saved that page last.

Does this help?

yes,i will update in PR !

LongTrong-exe commented 2 years ago

@jmaas-onoffice I have some cases that need clarification. What will happens to them?

  1. Page content blank and ACF content is detail shortcode image

  2. Page content have text but not onOffice shortcode and ACF content is detail shortcode image

  3. Page content and ACF content have 2 different onOffice shortcodes. Which will take precedence, or take both? image

  4. Same shortcode in both places

jmaas-onoffice commented 2 years ago

@LongTrong-exe I'm not sure I understand what the problem is. 😅

I assume for each case you have more than one option in mind and you don't know which one to choose. Can you tell me which options you are thinking of for each case?

LongTrong-exe commented 2 years ago

On master, I tried case (1.), (2.) and the page content is empty. And I don't know if this is a bug or not?

image image

image image

LongTrong-exe commented 2 years ago

@jmaas-onoffice Just to be sure, can you explain to me what the ACF plugin does? :sweat_smile:

jmaas-onoffice commented 2 years ago

Ah, I think I understand the problem!

So ACF is basically a form builder for WordPress. It gives you more control over what the users can put on the page. Maybe this video helps get a feeling for how it is used to build a page. So it let's you say what information can be on a page, then users can fill out that information, and you can use that information in the theme.

We are using ACF on all our websites, so we want it to work well with the plugin. Unfortunately, ACF stores the information in post meta fields instead of the post content, which breaks some things like WordPress's built-in shortcode detection. That's why we have to do some additional things to detect the shortcode in ACF.

I think the problem for testing is that you need to actually output the ACF content in your theme. So in this case, you have put the shortcode in an ACF field, but you probably do not have a theme that gets that data and uses it. That's why nothing happens. In a "real" case, the theme would have some code that sees that the shortcode is used and outputs it on the page.

All in all, what we can test for is that the detail page is recognized correctly. That means the correct page should be linke as "Detail view is in use on page ..." and the estate lists should link to the correct page.

I know this is a bit complicated, do you have more questions? :)

LongTrong-exe commented 2 years ago

Oh, I understand

jmaas-onoffice commented 2 years ago

If you have any more questions, feel free to ask! :)