• Resolved Ov3rfly

    (@ov3rfly)


    Some suggestions and ideas for a future release:

    1. Add a plugin-related prefix to variable php_params in wp_localize_script() and jrsm-jquery.js as described in docs, e.g. jrsm_params or jrsmParams.

    2. The extra call of wp_enqueue_script('jquery'); is not nesessary as array( 'jquery' ) is listed as $deps within wp_enqueue_script( 'jrsm-jquery' ... ) call, see also docs.

    3. If empty links are shown, could you add an additional option which makes it possible to add the prop disabled to these empty links <option> tags?

    4. In admin.php:48 the string 'Main Settings' is not yet localized.

    Thanks for the great plugin!

    https://www.ads-software.com/plugins/jquery-responsive-select-menu/

Viewing 10 replies - 1 through 10 (of 10 total)
  • Thread Starter Ov3rfly

    (@ov3rfly)

    Made a german translation, but it didn’t load, some more suggestions / bug fixes:

    5. Use add_action( 'plugins_loaded', 'jrsm_init' ); instead of plugins_init for loading the language files, see also docs.

    6. Rename the language files to match the text domain which is 'jrsm' in this case.

    If your plugin’s text domain is “my-plugin” the Danish .mo and.po files should be named “my-plugin-da_DK.mo” and “my-plugin-da_DK.po”

    7. The current .pot file is not complete yet, the entry for option Hide Empty Links is missing.

    Here an archive based on the current .pot file with the renamed spanish and new german language files, feel free to use as you like, link expires in 90 days from today.
    https://wikisend.com/download/273428/jquery-responsive-select-menu-languages.zip

    Again thanks for the great plugin!

    Plugin Author Mickey Kay

    (@mcguive7)

    Wow, thanks for the heads up on all these things!

    I’ve implemented all the fixes you recommend, including your translation (I took the liberty of Google translating a string I didn’t have in the old .pot file as well). I’ve tested and all translations seem to be working now. Thanks again!

    As for the “disabled” <option> feature, before I implement that, can you tell me why you would want them in there? I can’t think of a scenario when this would be a value to me. Thanks!

    Thread Starter Ov3rfly

    (@ov3rfly)

    In a current project all top-level menu items are “#” and styled with a cursor:default; instead of a pointer. It would be nice to have a “non clickable” effect within the mobile dropdown as well.

    I currently add the disabled prop to “#” options with an own javascript after the menu has been created. Just thought maybe others have a similar problem and would like an additional setting for this.

    Btw. starting with jQuery 1.6 it is recommended to use prop() for 'selected' and similar, details here, but after quite some discussions backward compatibility for attr() was provided again in 1.6.1.

    Plugin Author Mickey Kay

    (@mcguive7)

    PS – forgot to mention that I released version 1.3.1 with your fixes in place. Thank you!

    Thread Starter Ov3rfly

    (@ov3rfly)

    Thanks. Found another small thing in the meantime:

    8. Currently .jquery class is added to dynamic css output and then is added to body during .ready() via js.

    This causes a short flash of the real menu in e.g. Android 4.2.2 stock browser before the display: none !important; is used.

    As a fix I have now removed the adding of '.jquery ' in dynamic css creation. The css is then effective immediately via the mediaquery.

    Is there a special reason why you added the extra class trigger in addition to the mediaquery?

    Plugin Author Mickey Kay

    (@mcguive7)

    This is in case JS is disabled.

    Thread Starter Ov3rfly

    (@ov3rfly)

    Thanks for explaination.

    Usually the trigger class is added to html before .ready() fires. This avoids the described “flashing” situation, here an interesting post about that approach:

    https://www.learningjquery.com/2008/10/1-way-to-avoid-the-flash-of-unstyled-content/

    Significantly, the .addClass() method is not inside $(document).ready(). If we had put it inside, we’d be back to square one.

    Plugin Author Mickey Kay

    (@mcguive7)

    Hi again, and thanks so much for the feedback. Happy to report all of these issues and requests should be addressed in version 1.4, which I just released. This includes the flash of unstyled content, and the new admin option to add the disabled property to placeholder menu items.

    Please have a look and let me know if there’s anything I missed. Thanks!

    Thread Starter Ov3rfly

    (@ov3rfly)

    Looks fine with 1.4, flash is gone, new admin option works perfect.

    A small issue is in line 88 of .js, the comma at the end of the line is too much for older browsers.

    prefix + ' ' + text,

    Here an archive with an adjusted .pot file for 1.4 and the new german language files which both include the new options, feel free to use as you like, link expires in 14 days from today.
    https://wikisend.com/download/470538/jquery-responsive-select-menu-1.4-lang-de_DE.zip

    Another thing for the languages, the extra es.ES files (with the dot between) can be removed, only the es_ES are used by WordPress.

    Thanks for the great plugin.

    Plugin Author Mickey Kay

    (@mcguive7)

    Thanks again! All fixed and translations updated in versions 1.4.1, just released. Thanks!!!

Viewing 10 replies - 1 through 10 (of 10 total)
  • The topic ‘Some suggestions’ is closed to new replies.