Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typo (LICENCE -> LICENSE), user-dismissible UI notice, missing i18n. #21

Merged
merged 1 commit into from
Jan 6, 2016

Conversation

fabacab
Copy link
Contributor

@fabacab fabacab commented Jan 6, 2016

The LICENCE file is renamed to LICENSE because the latter is the
correct English spelling of the word.

The admin notice upon plugin activation now supports a user-dismissible
button, available from WordPress 4.2 and higher and backwards-compatible
with previous versions of WordPress. For details, see:

https://make.wordpress.org/core/2015/04/23/spinners-and-dismissible-admin-notices-in-4-2/

Finally, the "Install now" string was not translatable/localizeable
because it was hardcoded into the admin UI.

The languages/messages.pot file is also updated, because many such
strings were missing from it.

The `LICENCE` file is renamed to `LICENSE` because the latter is the
correct English spelling of the word.

The admin notice upon plugin activation now supports a user-dismissible
button, available from WordPress 4.2 and higher and backwards-compatible
with previous versions of WordPress. For details, see:

https://make.wordpress.org/core/2015/04/23/spinners-and-dismissible-admin-notices-in-4-2/

Finally, the "Install now" string was not translatable/localizeable
because it was hardcoded into the admin UI.

The `languages/messages.pot` file is also updated, because many such
strings were missing from it.
@fabacab
Copy link
Contributor Author

fabacab commented Jan 6, 2016

I also noticed that there are a number of areas in the admin UI that I think could be improved, perhaps by using the built-in WordPress <table class="form-table"> but I refrained from making any major changes because I'm not sure if they're welcome.

Would you be open to receiving pull requests that focus on improving the user interface of the admin area?

benjaminpick added a commit that referenced this pull request Jan 6, 2016
Typo (`LICENCE` -> `LICENSE`), user-dismissible UI notice, missing i18n.
@benjaminpick benjaminpick merged commit 604f6d8 into yellowtree:master Jan 6, 2016
@benjaminpick
Copy link
Member

Thanks!
Now that the Wordpress plugin directory uses transifex I'm not sure if the POT-Updates are still necessary, will have a look into setting it up. Afterwards, feel free to add translations there.

UI Changes: yes they are welcome. If it goes beyond embelleshing things maybe a draft/sketch would make sense. But definitely, improving UI/Usability is an ongoing effort.

(Target audience: The user base consists mainly of developers who know what they are doing, but also some simple users who install this plugin as a compagnion of a different plugin. So they should understand it as well in order to keep my support effort low.)

BTW this is the reason why there is currently a hidden feature (several reverse proxies, see https://github.com/yellowtree/wp-geoip-detect/blob/109dee6125ccaa9564af061f27cb0d940d913e0d/api.php#L162) because I couldn't come up with an simple to use & simple to develop way of adding this to the Options UI. The issue is that not all users will know the IPs of their reverse proxies, and maybe even don't know if IPv6 is used at all. On the other hand, auto-detecting them without asking the user is a security risk.

@fabacab
Copy link
Contributor Author

fabacab commented Jan 6, 2016

Now that the Wordpress plugin directory uses transifex I'm not sure if the POT-Updates are still necessary

Wait, really? I can not find information about this. Can you elaborate?

UI Changes: yes they are welcome. If it goes beyond embelleshing things maybe a draft/sketch would make sense. But definitely, improving UI/Usability is an ongoing effort.

Okay, good to know. I am a developer of other plugins and one of my recent projects needs geolocation based on IP addresses, so that's how I found your plugin.

I'm currently experimenting with using your plugin to provide geo-IP support and will happily contribute whatever improvements I can make as I explore your plugin.

Is GitHub the best way to ask you clarifying questions about the plugin or do you prefer another way, like email? Thanks for the work you put into this. :)

@benjaminpick
Copy link
Member

On 06.01.2016 18:15, Meitar M. wrote:

Now that the Wordpress plugin directory uses transifex I'm not sure
if the POT-Updates are still necessary

Wait, really? I can not find information about this. Can you elaborate?

See the announcement here ...

https://make.wordpress.org/plugins/2015/12/06/plugin-translations-for-all-plugins/

(Not so sure if its transifex behind the scenes but something like that.)

You are very welcome to contribute. If you change or add features, you
might need to learn some phpunit (coverage is not 100% but each feature
should have some tests, if feasable. Currently I do not do UI testing
(behat & co) but if you want you can add it.)

I have started work on an AJAX API (see #19) but currently am unable to
invest time in this.

Yes, plugin bugs & features should be coordinated here for posterity's
sake. If you have more general questions you can contact me via email.

@benjaminpick
Copy link
Member

The is-dismissible doesn't work as-is, because the notice will be shown again on the next request. Could you add an AJAX request to the current dismiss URL?

@fabacab
Copy link
Contributor Author

fabacab commented Jan 7, 2016

The is-dismissible doesn't work as-is, because the notice will be shown again on the next request.

The purpose of is-dismissible is to "close" the admin notice for the current screen only.

@fabacab
Copy link
Contributor Author

fabacab commented Jan 7, 2016

Also, for what it's worth, after a close read of the "WordPress translations for plugins announcement," I think they are using their own translation engine, GlotPress, rather than Transifex. I don't know what this means for the .pot file in plugins, but I guess that we will still need to include updated copies of them when our plugins change. Although, if not, cool. :)

@fabacab fabacab deleted the minor-ui-and-i18n-fixes branch January 9, 2016 09:40
@benjaminpick
Copy link
Member

I have released your fixes and the plugin can be translated now. If you are interested to become the admin of a locale let me know.

@fabacab
Copy link
Contributor Author

fabacab commented Feb 5, 2016

I only speak English well enough to be a translator. :( Thanks for releasing the updates. I haven't had time to do more work on this plugin because my other priorities took precedence, but when my work on those projects come back around to needing geo IP detection, I'll upstream any improvements I can make on your code to you.

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