Skip to content

Conversation

@roidayan
Copy link
Contributor

Browse is not part of the Wordpress translatation file so it doesn't get
translated. Choose File is and will be translated according to the
langauge Wordpress is set.

Signed-off-by: Roi Dayan [email protected]

@roidayan
Copy link
Contributor Author

The suggestion here is because Browse is being translated with the default Wordpress text domain so by default it should be a translatable string. When I changed language I saw Browse kept untranslated and looked in the Wordpress core translation strings what is available.
It doesn't block anyone using this class to use something else. i.e. I use "Choose Image" as I know I want an image which is also translated.
Maybe it also fits as it opens the media browser anyway.

Maybe should also separate between type=file and type=media/image and have 2 callbacks.

@tareq1988
Copy link
Owner

For file uploads, we should have a configurable label. Right now it's hardcoded and the change you made also hardcoded as well. Instead we should pass the text as a parameter and it'll fallback to __( 'Choose File' ); if none given.

@roidayan roidayan force-pushed the refactor_browse_string branch from 2873f3e to 9a84b80 Compare May 19, 2015 12:23
@roidayan
Copy link
Contributor Author

ok. added option to pass button label and the default is Choose File which is translated according to Wordpress language.
Also updated the example with this.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please use ternary operator instead of using a big if/else block?

@roidayan
Copy link
Contributor Author

did it at first but it was pretty big by itself so didn't think it's needed as it didn't add readability. but ok.

@roidayan
Copy link
Contributor Author

its pretty big I think if-else is more readable for this case. do you still want it like this ?

$label = ( isset( $args['options'] ) && isset( $args['options']['button_label'] ) ) ?
                        $label = $args['options']['button_label'] :
                        $label = __( 'Choose File' );

@tareq1988
Copy link
Owner

Checking only isset( $args['options']['button_label']) should work, no?

@roidayan
Copy link
Contributor Author

i'll check if it doesn't give warning in debug

@roidayan
Copy link
Contributor Author

only isset( $args['options']['button_label']) works good. i'll update the commit. thanks

@roidayan roidayan force-pushed the refactor_browse_string branch from 9a84b80 to 6557b84 Compare May 19, 2015 13:30
@roidayan
Copy link
Contributor Author

updated

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be like this:

$label = isset( $args['options']['button_label'] ) ? $args['options']['button_label'] : __( 'Choose File' );

Don't need to use $label on the ternary operator.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh damn. didn't notice it.

Browse is not part of the Wordpress translatation file so it doesn't get
translated. Choose File is and will be translated according to the
langauge Wordpress is set.

Also add option to pass button label.

Signed-off-by: Roi Dayan <[email protected]>
@roidayan roidayan force-pushed the refactor_browse_string branch from 6557b84 to 3bb2e13 Compare May 19, 2015 19:55
tareq1988 added a commit that referenced this pull request May 19, 2015
File "Browse" string changed to "Choose File"
@tareq1988 tareq1988 merged commit 9901313 into tareq1988:master May 19, 2015
@roidayan roidayan deleted the refactor_browse_string branch May 19, 2015 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants