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

Improve shared session tooltip #16514

Merged
merged 6 commits into from
Jun 5, 2017

Conversation

andrepereiradasilva
Copy link
Contributor

@andrepereiradasilva andrepereiradasilva commented Jun 4, 2017

Pull Request for Issue #16485.

Summary of Changes

Improves shared session tooltip to inform users that shared session will not work when Force HTTPS is set to Administrator only.

Testing Instructions

Language review.

Expected result

Used informed.

Actual result

Used not informed.

Documentation Changes Required

None.

@@ -194,7 +194,7 @@ COM_CONFIG_FIELD_SESSION_HANDLER_DESC="The mechanism by which Joomla identifies
COM_CONFIG_FIELD_SESSION_HANDLER_LABEL="Session Handler"
COM_CONFIG_FIELD_SESSION_TIME_DESC="Auto log out a User after they have been inactive for the entered number of minutes. Do not set too high."
COM_CONFIG_FIELD_SESSION_TIME_LABEL="Session Lifetime"
COM_CONFIG_FIELD_SHARED_SESSION_DESC="When enabled, a user's session is shared between the frontend and administrator sections of the site. Note that changing this value will invalidate all existing sessions on the site."
COM_CONFIG_FIELD_SHARED_SESSION_DESC="When enabled, a user's session is shared between the frontend and administrator sections of the site. Note that changing this value will invalidate all existing sessions on the site and that, due to the use secure cookies, shared session will not work when \"Force HTTPS\" option is set to \"Administrator Only\"."
Copy link
Member

Choose a reason for hiding this comment

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

... when the "Force HTTPS" option is set to "Administrator Only"."

I think you should add the "the".

@andrepereiradasilva
Copy link
Contributor Author

hound — No violations found. Woof! ??? 💯

@Bakual
Copy link
Contributor

Bakual commented Jun 4, 2017

See #16515

@richard67
Copy link
Member

I still think that "... will not work when the "Force HTTPS" option is set to ..." is better English than "... will not work when "Force HTTPS" option is set to ...".

@richard67
Copy link
Member

I have tested this item ✅ successfully on 69975c2

The changed text is shown in the tool tip for the "Shared Sessions" value when this PR is applied.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16514.

@@ -194,7 +194,7 @@ COM_CONFIG_FIELD_SESSION_HANDLER_DESC="The mechanism by which Joomla identifies
COM_CONFIG_FIELD_SESSION_HANDLER_LABEL="Session Handler"
COM_CONFIG_FIELD_SESSION_TIME_DESC="Auto log out a User after they have been inactive for the entered number of minutes. Do not set too high."
COM_CONFIG_FIELD_SESSION_TIME_LABEL="Session Lifetime"
COM_CONFIG_FIELD_SHARED_SESSION_DESC="When enabled, a user's session is shared between the frontend and administrator sections of the site. Note that changing this value will invalidate all existing sessions on the site."
COM_CONFIG_FIELD_SHARED_SESSION_DESC="When enabled, a user's session is shared between the frontend and administrator sections of the site. Note that changing this value will invalidate all existing sessions on the site and that, due to the use secure cookies, shared sessions will not work when the \"Force HTTPS\" option is set to \"Administrator Only\"."
Copy link
Contributor

Choose a reason for hiding this comment

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

please make this a new sentence. long sentences are harder to understand and introduce translation issues. It is also enough to simply add

"This will not work when Force https is set to administrator onlly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@brianteeman can i ask you to put a better english? thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

+COM_CONFIG_FIELD_SHARED_SESSION_DESC="When enabled, a user's session is shared between the frontend and administrator sections of the site. Note that changing this value will invalidate all existing sessions on the site. Shared sessions is not available when the "Force HTTPS" option is set to "Administrator Only"."

Copy link
Contributor

Choose a reason for hiding this comment

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

Shared session is not or Shared sessions are not?

@andrepereiradasilva
Copy link
Contributor Author

changed thanks @brianteeman

@richard67
Copy link
Member

I have tested this item ✅ successfully on f5ebae4


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16514.

@@ -194,7 +194,7 @@ COM_CONFIG_FIELD_SESSION_HANDLER_DESC="The mechanism by which Joomla identifies
COM_CONFIG_FIELD_SESSION_HANDLER_LABEL="Session Handler"
COM_CONFIG_FIELD_SESSION_TIME_DESC="Auto log out a User after they have been inactive for the entered number of minutes. Do not set too high."
COM_CONFIG_FIELD_SESSION_TIME_LABEL="Session Lifetime"
COM_CONFIG_FIELD_SHARED_SESSION_DESC="When enabled, a user's session is shared between the frontend and administrator sections of the site. Note that changing this value will invalidate all existing sessions on the site."
COM_CONFIG_FIELD_SHARED_SESSION_DESC="When enabled, a user's session is shared between the frontend and administrator sections of the site. Note that changing this value will invalidate all existing sessions on the site. Shared sessions is not available when the \"Force HTTPS\" option is set to \"Administrator Only\"."
Copy link
Contributor

Choose a reason for hiding this comment

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

Having thought more on this it is possible that a translator will not correctly understand this - so I will have another attempt at rewording this later - please be patient and do not merge

Copy link
Contributor

Choose a reason for hiding this comment

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

When the "Force HTTPS" option is set to "Administrator Only" this is not available.

Copy link
Contributor

Choose a reason for hiding this comment

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

that should be easier

Copy link
Contributor

@Quy Quy Jun 4, 2017

Choose a reason for hiding this comment

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

I would rephrase it This is not available when the "Force HTTPS" option is set to "Administrator Only". to let the user know sooner than later in the sentence.

@andrepereiradasilva
Copy link
Contributor Author

ok corrected. please check

@richard67
Copy link
Member

I have tested this item ✅ successfully on 719fc31


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16514.

1 similar comment
@Quy
Copy link
Contributor

Quy commented Jun 4, 2017

I have tested this item ✅ successfully on 719fc31


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16514.

@ghost
Copy link

ghost commented Jun 4, 2017

RTC after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jun 4, 2017
@wilsonge wilsonge merged commit d4e58b9 into joomla:staging Jun 5, 2017
@wilsonge wilsonge added this to the Joomla 3.7.3 milestone Jun 5, 2017
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jun 5, 2017
@andrepereiradasilva andrepereiradasilva deleted the improve-tooltip branch June 5, 2017 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants