OpenDevelopmentMekong / wp-odm_tabular_pages

Internal ODM Wordpress plugin for exposing a page template for tabular pages
https://opendevelopmentmekong.net
GNU General Public License v3.0
0 stars 2 forks source link

Impl 129 #139

Closed Huyeng closed 7 years ago

Huyeng commented 7 years ago

Fixes #129

acorbi commented 7 years ago

@Huyeng I have deployed this branch on PP, commit 1 code enhancement (please see) and also identified the following warning on https://pp.opendevelopmentmekong.net/wp-admin/post.php?post=4085766&action=edit and https://pp.opendevelopmentcambodia.net/wp-admin/post.php?post=98002&action=edit

screenshot from 2017-02-20 11-52-52

Can you please fix it so I can continue my review? Also, I noticed that you are managing the "localization" of the options differently as we were doing before. You are using now [en] or [localization] for managing them while on other options such as _attributes_values_mapping we are storing a separate value _attributes_values_mapping_localization. Please unify this implementation otherwise is too confusing...

Huyeng commented 7 years ago

I will fix it now. I don't think it will confuse. The _attributes_values_mapping is using to match the key/id of CKAN with the label on front-end. The _attributes_filters_group_list[en]/[localization], those are just the label to show up on the site. The labels can change to update as the editor needs both Khmer and English.

screenshot_5

acorbi commented 7 years ago

@Huyeng What I mean it can be confusing is that the same functionality is implemented differently. this could be confusing for other developers looking at the code. thanks for fixing the warning, ping me when it is ready and I will continue testing

Huyeng commented 7 years ago

Ok. Which one do you prefer, array or variable?