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

Better messages #3827

Closed
wants to merge 10 commits into from
Closed

Better messages #3827

wants to merge 10 commits into from

Conversation

jancborchardt
Copy link
Member

Improving the error messages on installation by linking to the documentation. The documentation links should ideally be moved to the defaults.php, can you help with that @schiesbn? Is the only thing needing change in the doc url .org to .com?

Also changed the WebDAV sentence to link to the docs, same here.

cc @DeepDiver1975 regarding #3819

@DeepDiver1975
Copy link
Member

Up to now we did not translate these setup messages.
Is now the time to do so?

@bantu
Copy link

bantu commented Jun 24, 2013

Don't like the specification of target for links. Users should be able to decide on their own where links open.

@jancborchardt
Copy link
Member Author

@bantu it’s a web app, links should not load in the same window.

@jancborchardt
Copy link
Member Author

@DeepDiver1975 probably yes? What do you think?

@schiessle
Copy link
Contributor

added OC_Defaults call for the base url, otherwise it looks good to me 👍

@@ -104,7 +104,7 @@
<fieldset class="personalblock">
<legend><strong><?php p($l->t('WebDAV'));?></strong></legend>
<code><?php print_unescaped(OC_Helper::linkToRemote('webdav')); ?></code><br />
<em><?php p($l->t('Use this address to connect to your ownCloud in your file manager'));?></em>
<em><?php print_unescaped($l->t(sprintf('Use this address to <a href="%s/server/5.0/user_manual/files/files.html" target="_blank">access your Files via WebDAV</a>', OC_Defaults::getDocBaseUrl()) ));?></em>
Copy link
Member

Choose a reason for hiding this comment

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

this sprintf inside t() will not work.
t() can take arguments an interpolate the translated string

@DeepDiver1975
Copy link
Member

@DeepDiver1975 probably yes? What do you think?

let's leave it as it is - the admin will see this once/never.

@jancborchardt
Copy link
Member Author

Can we get another tester here?

@jancborchardt
Copy link
Member Author

(cc @bantu @DeepDiver1975 ;)

@schiessle
Copy link
Contributor

This should go in for 5.0.8 and the EE release, right @jancborchardt .

cc @DeepDiver1975 @bantu @VicDeo @butonic

@@ -6,6 +6,8 @@

<?php $defaults = new OC_Defaults(); // initialize themable default strings and urls ?>

<?php $defaults = new OC_Defaults(); // initialize themable default strings and urls ?>
Copy link
Member

Choose a reason for hiding this comment

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

initialized twice

@VicDeo
Copy link
Member

VicDeo commented Jul 5, 2013

@schiesbn I'm fine with that 👍
Tested
Please consider changing the following messages accordingly
https://github.com/owncloud/core/blob/master/lib/base.php#L179
https://github.com/owncloud/core/blob/master/lib/config.php#L186

@schiessle
Copy link
Contributor

Merged: 9b9ea7c

@schiessle schiessle closed this Jul 8, 2013
@schiessle
Copy link
Contributor

backported to stable5: 54a1239

@jancborchardt jancborchardt deleted the better-messages branch July 26, 2013 08:38
@lock lock bot locked as resolved and limited conversation to collaborators Aug 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants