feat(www): add alternate link in farsi #1442

ohnorobo · 2022-10-19T17:47:20Z

Add an additional message/link to the empty client screen and new server dialog in Farsi (and not on ios.)

This is just manually adding the new strings in English and Farsi. no other translations exist yet, but it doesn't matter because they're only displayed in Farsi. Eventually we will get additional translations which might be helpful if we ever want to expand the languages for this in the future.

Tested (tests done in EN / FA / IT (to represent all other languages which don't have the translated string)):

viewing the zero view
viewing the server add view
inputting a bad ss key in the server add view (to switch to the bad ss key message) and then removing the key to switch back to the access message
@kevindamm-jigsaw tested that theios/osx exclusion part of this works.
that messages are re-localized/properties recomputed correctly on language change
that the zero page / page view with servers still renders correctly without hidden

Zero view FA:

Server add view FA:

fortuna · 2022-10-24T14:00:21Z

src/www/ui_components/add-server-view.js

          inner-h-t-m-l="[[localize('server-create-your-own', 'breakLine', '<br/>', 'openLink', '<a href=https://s3.amazonaws.com/outline-vpn/index.html>', 'closeLink', '</a>')]]"
        ></div>
+        <div
+          id="addServerFooterAlt"
+          hidden$="[[!showAltAccessMessage]]"


This approach is problematic because the both templates will be rendered. It pollutes the DOM, causes subtle bugs if you reuse ids and one of them may crash with missing messages.

Use instead: https://polymer-library.polymer-project.org/3.0/docs/devguide/templates#dom-if

After chatting with @jyyi1 I switched this to use dom-if in servers_view/index.ts but not here. _inputInvalidChanged here is already relying on the hidden property in an inadvisable way. But we can't stop using it here without breaking or entirely refactoring that code.

Welcome to developing in the client @ohnorobo !!!!

@fortuna I have a proposal in my comment below, which can simplify the logic in _inputInvalidChanged. And you will also be able to use dom-if as well because we will not reference any specific elements in the function. But it requires more code changes. Depending on the workload, we can either refactor it later or do it now.

src/www/ui_components/add-server-view.js

fortuna · 2022-10-24T14:03:37Z

src/www/views/servers_view/index.ts

@@ -108,8 +108,14 @@ Polymer({
        </div>
        <div
          class="footer subtle"
+          hidden$="[[showAltAccessMessage]]"


Ditto. Also for the changes below.

I've also replaced two unrelated uses of hidden in this component with dom-if.

src/www/ui_components/app-root.js

src/www/messages/en.json

src/www/ui_components/add-server-view.js

fortuna · 2022-10-24T19:27:02Z

src/www/ui_components/app-root.js

@@ -540,6 +540,10 @@ export class AppRoot extends mixinBehaviors([AppLocalizeBehavior], PolymerElemen
      toastUrl: {
        type: String,
      },
+      showAltAccessMessage: {
+        type: Boolean,
+        computed: '_computeShowAltAccessMessage()',


You need to express the dependency on the language so it gets recomputed on change.

Suggested change

computed: '_computeShowAltAccessMessage()',

computed: '_computeShowAltAccessMessage(language)',

Please test the behavior change on language change.

done, this definitely fixed some of the weirdness on language change

jyyi1 · 2022-10-24T21:10:33Z

src/www/ui_components/add-server-view.js

      this.$.invalidAccessKeyFooter.hidden = false;
    } else {
-      this.$.addServerFooter.hidden = false;
+      this.$.addServerFooter.hidden = false || this.showAltAccessMessage;


@daniellacosse I feel this logic is not prefect. We may introduce a new boolean property accessKeyValid in this element, and update it in this function. In this case, we don't need to reference any specific elements, those elements can bind to this new property, for example, <div id="addServerFooterAlt" hidden$="[[!accessKeyValid || !showAltAccessMessage]]" />. Just a potential action item, no need to change it in this PR, but we may refactor it later.

That aligns with my suggestion. A variable that indicates whether to show the invalid key error, or the server suggestion message. 👍🏼

Done.

Added invalidAccessKeyInput property. I know negative booleans are bad, but the logical split here is (no access key input, valid access key input) vs (invalid access key input) so not using the negative made the logic more confusing.

Unfortunately no boolean logic except !var is allowed in html, so switching to dom-if required adding two new computed properties and two new functions to contain the boolean logic. I've switched the name of the passed property to useAltAccessMessage everywhere to avoid confusion with the show... properties that are being computed in this element.

src/www/messages/en.json

fortuna

Thanks for the changes!

jyyi1

LGTM, thanks.

jyyi1 · 2022-10-25T15:21:46Z

src/www/ui_components/add-server-view.js

@@ -298,15 +320,21 @@ Polymer({
    var input = event.target;
    input.toggleClass('input-invalid', input.invalid);
    if (input.invalid) {
-      this.$.addServerFooter.hidden = true;
-      this.$.invalidAccessKeyFooter.hidden = false;
+      this.invalidAccessKeyInput = true;


minor this.invalidAccessKeyInput = input.invalid

github-actions bot added the size/XS label Oct 19, 2022

ohnorobo force-pushed the external-link branch 2 times, most recently from 8f4ab9c to 323d9b9 Compare October 19, 2022 18:22

ohnorobo added a commit that referenced this pull request Oct 19, 2022

feat(www): add external link to empty main page in english #1442

a4deeb7

ohnorobo force-pushed the external-link branch from 323d9b9 to a4deeb7 Compare October 19, 2022 18:32

daniellacosse changed the title ~~add link in english/farsi~~ feat(www): add link in english/farsi Oct 19, 2022

ohnorobo mentioned this pull request Oct 20, 2022

feat(www): add external link to server add messages farsi #1446

Closed

ohnorobo changed the title ~~feat(www): add link in english/farsi~~ feat(www): add alternate link in farsi Oct 24, 2022

ohnorobo force-pushed the external-link branch 5 times, most recently from fbec6fd to 4a76996 Compare October 24, 2022 13:45

ohnorobo requested a review from a team October 24, 2022 13:52

fortuna requested changes Oct 24, 2022

View reviewed changes

ohnorobo marked this pull request as ready for review October 24, 2022 16:52

fortuna requested changes Oct 24, 2022

View reviewed changes

src/www/messages/en.json Show resolved Hide resolved

fortuna requested changes Oct 24, 2022

View reviewed changes

jyyi1 reviewed Oct 24, 2022

View reviewed changes

fortuna requested changes Oct 24, 2022

View reviewed changes

src/www/messages/en.json Show resolved Hide resolved

github-actions bot added size/S and removed size/XS labels Oct 25, 2022

ohnorobo force-pushed the external-link branch from aeb07a5 to 1fca085 Compare October 25, 2022 12:35

ohnorobo requested a review from fortuna October 25, 2022 13:05

ohnorobo added 6 commits October 25, 2022 13:11

add alternative access message in farsi

6c0c2a4

add osx exclusion

aafb17f

pass computed property instead of language and platform

7ba28ae

use dom-if templates in servers_view

b37122a

remove remaining uses of hidden

64051cf

update farsi string

d854db6

ohnorobo force-pushed the external-link branch from f6198f0 to d854db6 Compare October 25, 2022 13:11

fortuna approved these changes Oct 25, 2022

View reviewed changes

jyyi1 approved these changes Oct 25, 2022

View reviewed changes

nit

a51bb29

ohnorobo merged commit 0d32b1e into master Oct 25, 2022

sbruens deleted the external-link branch March 5, 2024 22:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(www): add alternate link in farsi #1442

feat(www): add alternate link in farsi #1442

ohnorobo commented Oct 19, 2022 •

edited

Loading

fortuna Oct 24, 2022

ohnorobo Oct 24, 2022

daniellacosse Oct 24, 2022

jyyi1 Oct 24, 2022

ohnorobo Oct 25, 2022

fortuna Oct 24, 2022

ohnorobo Oct 24, 2022

ohnorobo Oct 25, 2022 •

edited

Loading

fortuna Oct 24, 2022

fortuna Oct 24, 2022

ohnorobo Oct 25, 2022

jyyi1 Oct 24, 2022 •

edited

Loading

fortuna Oct 24, 2022

ohnorobo Oct 25, 2022

fortuna left a comment

jyyi1 left a comment

jyyi1 Oct 25, 2022 •

edited

Loading

ohnorobo Oct 25, 2022

	computed: '_computeShowAltAccessMessage()',
	computed: '_computeShowAltAccessMessage(language)',

feat(www): add alternate link in farsi #1442

feat(www): add alternate link in farsi #1442

Conversation

ohnorobo commented Oct 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ohnorobo Oct 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jyyi1 Oct 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fortuna left a comment

Choose a reason for hiding this comment

jyyi1 left a comment

Choose a reason for hiding this comment

jyyi1 Oct 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ohnorobo commented Oct 19, 2022 •

edited

Loading

ohnorobo Oct 25, 2022 •

edited

Loading

jyyi1 Oct 24, 2022 •

edited

Loading

jyyi1 Oct 25, 2022 •

edited

Loading