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

In old templates, parking lots are still displayed as "Array, Array, Array" #413

Closed jmaas-onoffice closed 1 year ago

jmaas-onoffice commented 1 year ago

Parking lots not backwards compatible

With #105, when using the updated templates, the parking lots are displayed correctly.

However, when using an old template (basically any template except those from #105), the parking lots are still displayed as "Array, Array, Array":

image

Make backwards compatible by changing the value in the loop

In #398, the idea was that we can fix this by changing how the value of the parking lots is stored for this loop: https://github.com/onOffice-Web-Org/oo-wp-plugin/blob/5f95ee239c457257d9f759f1a69279c75dcc8c56/templates.dist/estate/default.php#L100-L115

Since older templates just run through this loop, if we set this $value to the text display of the parking lots, then the parking lots would work for older templates.

Keep getValueRaw()

However, we want to keep getValueRaw() which the updated templates from #105 use, because that makes it possible to customize how the parking lots are displayed.

hungnc89 commented 1 year ago

@jmaas-onoffice are you using similar estate template ?

hungnc89 commented 1 year ago

@jmaas-onoffice Did you mean want I bring that code in default template to apply for all others template and make sure it's will be show right value ? right

jmaas-onoffice commented 1 year ago

@hungnc89 I'm not sure what you mean...

How I test it: I have an old installation and use the personalized template there.

You can basically just copy the default templates from master (those without the changes of #105) and use those as personalized templates to test the change from #105. They should currently show "Array, Array, Array".

hungnc89 commented 1 year ago

@jmaas-onoffice ah ok I understand

dai-eastgate commented 1 year ago

@jmaas-onoffice I fixed, you can watch video and give me feedback

https://user-images.githubusercontent.com/106214469/212250885-15aa3d17-63c9-41f3-9e18-2e5f9882462d.mp4

jmaas-onoffice commented 1 year ago

That looks good.

Is is still possible to access the raw values in templates? For example if a user wants to display it as "Carports: 2 @ RON 12,777" instead of the default formatting?

hungnc89 commented 1 year ago

@jmaas-onoffice currently we only display value formatted, not show raw value, if the user wants to show raw value instead of value formatted, I think we will add the option checkbox type for the user to set display raw value or value formatted, what do you think?

hungnc89 commented 1 year ago

@jmaas-onoffice can you help me ?

@jmaas-onoffice currently we only display value formatted, not show raw value, if the user wants to show raw value instead of value formatted, I think we will add the option checkbox type for the user to set display raw value or value formatted, what do you think?

jmaas-onoffice commented 1 year ago

@hungnc89 Sorry for the delayed answer!

  1. Are you suggesting that we add a checkbox somewhere to switch between the two?
  2. Is there currently no way to show the text by default (like it is now) and still access the object using getValueRaw()?
hungnc89 commented 1 year ago

@jmaas-onoffice

  1. yes, I think so, we need a checkbox to switch between
  2. yes
jmaas-onoffice commented 1 year ago

Ok, I'll have a look.

dai-eastgate commented 1 year ago

@jmaas-onoffice I added a checkbox, you can watch video and give me feedback

https://user-images.githubusercontent.com/106214469/215701459-151a264c-bf65-436f-a352-962181eab4a6.mp4

jmaas-onoffice commented 1 year ago

@dai-eastgate There was a misunderstanding. My comment wasn't very clear, sorry for that.

We currently do not want a checkbox. I'll discuss with Linh in our weekly tomorrow what the current plan for this is. :)

jmaas-onoffice commented 1 year ago

Because it is currently not possible to have the text as default and access the raw object on demand for the same field, let's first focus on fixing the parking lots by using the fixed text.

As far as I can tell, this is already working as desired in #105.

image