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

Add outdated_translation variable #220

Merged
merged 5 commits into from
Sep 5, 2019
Merged

Add outdated_translation variable #220

merged 5 commits into from
Sep 5, 2019

Conversation

huey735
Copy link
Contributor

@huey735 huey735 commented Aug 26, 2019

On @m52go's advice I changed the construction of the outdated
translation alert tool to be spefic to the page instead of
the language. I added a "outdated_translation" variable to
the front matter of the relevant translated pages. They'll
have the values 'true' or 'false' according to the state.

On @m52go's advice I changed the construction of the outdated
translation alert tool to be spefic to the page instead of
the language. I added a "outdated_translation" variable to
the front matter of the relevant translated pages. They'll
have the values 'true' or 'false' according to the state.
@m52go
Copy link
Contributor

m52go commented Aug 26, 2019

Thanks Huey -- much better PR.

Another thing we need to make sure to test is how the site appears on mobile, since the whole site is built to be responsive.

Making the font smaller won't work...we need to figure something else out.

Screenshot from 2019-08-26 13-52-11


<!-- Outdated translation alert tool tag-->
{% if page.outdated_translation %}
<span class="navbar-text" style="color: orange;">Outdated translation</span>
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest to add a link toTransifex, so that potential contributors know where to go if they want to help translate the page. The message could be something like Outdated translation - [Help us to improve it!](https://www.transifex.com/bisq/bisq-website/).

Would be also cool to have the phrase translatable imho, so that when somebody is using another language the warning will be in their language.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@erciccione what do you mean by message?

Copy link
Contributor

Choose a reason for hiding this comment

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

The content of the warning (that now is only Outdated translation)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, for that I think we'll have to move it from the navbar. There's not enough space there as it is.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think the navbar is the best place for that warning. I would put it in a dedicated snippet under the navbar. That way would be easier to see for the user, we would have more space so we can be more descriptive and styling form mobile would be easier to manage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@erciccione how about this, following the standard warning sign palette. Orange text on green background doesn't stand out that much.
outdated2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The black text however doesn't imply that there's an hyperlink to be clicked.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not great with colours so, keep that in mind :P, but i personally think this solution is better than simple text. What about having the hyperlink underlined? or underlined on hover?

Copy link
Contributor Author

@huey735 huey735 Sep 4, 2019

Choose a reason for hiding this comment

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

@ericcione now it looks like this when hovered
outdated3

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess could be fine. Better wait @m52go's input on this

@m52go m52go mentioned this pull request Aug 29, 2019
@m52go
Copy link
Contributor

m52go commented Sep 3, 2019

Now I'm kind of debating the need for this mechanism in the first place. The website isn't updated that often.

Major updates we should mostly know ahead of time and can plan to get the necessary translations ahead of time.

Minor updates probably don't justify having a page-wide flag...it's not that helpful to the user anyway because they won't know what's up-to-date and what's not.

@huey735
Copy link
Contributor Author

huey735 commented Sep 3, 2019

@m52go I still support this tool or some version of it.
In my mind the flag is to let the user know that they should refer to the English version for the up to date info. It also works as an ad to translators.

Move outdated_translation tool from _includes/main_nav_tr.html
to _layouts/page.html.
@m52go
Copy link
Contributor

m52go commented Sep 4, 2019

I am in favor of showing a message to encourage translators to contribute updates.

In that case, maybe it would be better to put a more subtle message toward the bottom (right above the site footer at the bottom of the page content) saying something to the effect "This translation is slightly out-of-date. Would you like to help?"

@erciccione
Copy link
Contributor

I wouldn't put it at the bottom. Users need to know if a document is up to date before they start reading it. That's better UX.

I would keep the warning at the top under the navbar, but i would make it more visible. Maybe i light green background?

Changed the text color to black and background-color to orange.
And on hover the text-decoration is underline and the color is
blue.
@m52go
Copy link
Contributor

m52go commented Sep 4, 2019

Could we do something a little more elegant? I don't think the current proposal goes well with the site's aesthetic.

For example:

Screenshot from 2019-09-04 13-19-09

@huey735
Copy link
Contributor Author

huey735 commented Sep 4, 2019

@m52go Am I right in assuming that that warning sign at the beginning of the sentence is the unicide U+26A0 and not an image?

@m52go
Copy link
Contributor

m52go commented Sep 4, 2019

Yes. This is exactly what I put in the if condition. Don't need any additional CSS either.

<p class="text-muted">⚠ This page contains some outdated translations. Help us improve <a href="https://www.transifex.com/bisq/bisq-website/">here</a>!</p>

@huey735
Copy link
Contributor Author

huey735 commented Sep 4, 2019

@m52go Cool, I had come down to something similar. Had span instead of p though.

Copy link
Contributor

@m52go m52go left a comment

Choose a reason for hiding this comment

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

This looks ready to go, thanks! Just one small question (see comment).

@m52go
Copy link
Contributor

m52go commented Sep 4, 2019

Notes:

  • the outdated_translation variables need to be added to the French pages, but this doesn't need to be done immediately (in this commit) as having no value is also falsey (which results in desired behavior)
  • with erciccione's Refactored "Statistics" page to make it more i18n friendly #221 merged, we should be able to get rid of the flag on all stats pages

@m52go
Copy link
Contributor

m52go commented Sep 5, 2019

ACK

@m52go m52go merged commit 4ea9d82 into bisq-network:master Sep 5, 2019
<!-- Outdated translation alert tool tag-->
<div class="col-md-12 order-md-1 text-md-left">
{% if page.outdated_translation %}
<p class="text-muted">⚠ This page contains some outdated translations. Help us improve <a href="https://www.transifex.com/bisq/bisq-website/">here</a>!</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't make links like this:
Click here for more information
Make links like this:
More information

@huey735 huey735 deleted the addOutdatedTranslationVariable branch September 8, 2019 15:28
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.

4 participants