hard-g / bp-braille Goto Github PK

You suggested server-side caching for Braille translations, which seems like a great idea. Not critical for launch, but if you've got the availability, go ahead and do it.

Be careful about invalidation. For forum items, you can probably use the 'clean_post_cache' hook. For messages, will have to find some reliable hook in BP's database layer.

The Braille plugin has a .braille class that it wraps around WP content that it translates to Braille. It uses this to apply Braille-specific CSS. Can we do the same? Perhaps just <span class="braille">...</span>.

It's not currently (easily) possible to unhook stuff like Groups\Settings::register() because the clases are initialized inside bootstrap(). (The theme we've got to integrate into has different UI requirements.) Could you rearchitect a bit so that the hooks are available from the global scope? Maybe a singleton App:

function bootstrap() {
    App::init();
}

// ...

namespace HardG\BpBraille;
class App {
    private function __construct() {}
    
    public static function get_instance() {
        static $instance;
        if ( empty( $instance ) ) {
            $instance = new self();
        }
        return $instance;
    }

    public static function init() {
        $instance = self::get_instance();
        $instance->messages_template = new Messages\Template( get_current_user_id() );
        $instance->messages_template->register();
        // ...
    }
}

which would mean that a theme could do the following:

remove_action( 'bp_after_group_details_admin', [ \HardG\BpBraille\App::get_instance()->groups_template, 'render' ] );

I don't care much about the details of the architecture. The pattern above is something I have used lately but it's imperfect, so I'm open to other suggestions.

Hey @Mamaduka - I just realized that this project needs to support PHP 5.3, so we can't use the [ 'foo' ] array syntax. Could you swap it for array( 'foo' )? Thanks, and sorry!

Switch from _e() to esc_html_e() etc.

Right now the whole of the plugin is only loaded if function_exists( 'get_braille' ). Under normal circumstances, this is perfectly fine :) But because Braille requires a separate library, I think we should probably have some sort of more robust check for the correct configuration. Otherwise I can pretty much guarantee that people will click the plugin on, and find that nothing works, and not understand why.

This is a problem that should probably be solved at the level of the Braille plugin, but I'm not 100% certain that they're accepting PRs (at least not yet). So I'd like to build some sort of check into bp-braille. Unfortunately, the LibLouis-Remoter.php library doesn't make it easy to do a test check - it calls die on failure (not even wp_die()!) instead of returning some sort of error. So, I'd suggest something like this (you can decide on the naming and the details):

function braille_works() {
    $configured = get_transient( ... );
    if ( false === $configured ) {
        $response = wp_remote_post( $liblouis, array( ... ) ); // see returnBrailleForString()
        $works =  ! request is failed
        set_transient( ... ) // for some reasonable amount of time
    }
}

If the Braille authors decide to accept pull requests, this can be repurposed into a PR for their plugin.

Don't load group settings if:

• bbPress is not present
• forums are not enable for a specific group

Could you please add a loading class and a spinner when the AJAX Braille call is taking place?