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

[VIVO-1839] - Use language name for selector instead of flags #165

Merged
merged 9 commits into from
Jun 22, 2020

Conversation

gneissone
Copy link
Member

@gneissone gneissone commented May 21, 2020

JIRA Issue: https://jira.lyrasis.org/browse/VIVO-1839

  • Other Relevant Links (Mailing list discussion, related pull requests, etc.)
    From Slack

Screen Shot 2020-05-21 at 1 08 38 PM

What does this pull request do?

Changes the language selector dropdown from displaying flag images to displaying the locale name.

Screen Shot 2020-05-21 at 12 56 37 PM Screen Shot 2020-05-21 at 12 56 52 PM

What's new?

Displays the locale name in the language selector (in that locale's language, e.g. the text for German will always say Deutsch regardless of which language is currently selected.

How should this be tested?

Build with changes. Confirm language selector still works as intended and languages are display as text, not images.

Additional Notes:

The label is generated from Java's Locale library, so the language must use a recognized locale code to work correctly. This matches the current behavior, which only made an exception for a special 'es-GO' translation that used to be included with VIVO (see

if (!"es_GO".equals(localeString) && // No complaint about bogus locale
).

Interested parties

@awoods

@gneissone gneissone changed the base branch from sprint-i18n to master May 21, 2020 19:32
@gneissone
Copy link
Member Author

If this is merged, a handful of flag images can be deleted from VIVO, VIVO language and Vitro language repos.
Screen Shot 2020-05-21 at 12 58 53 PM
Screen Shot 2020-05-21 at 12 59 28 PM
Screen Shot 2020-05-21 at 12 59 09 PM

@awoods
Copy link
Member

awoods commented May 22, 2020

Actually, with the fix of a trivial merge conflict in the file:
api/src/main/java/edu/cornell/mannlib/vitro/webapp/edit/n3editing/VTwo/AdditionsAndRetractions.java
.. I was able target this pull-request at the sprint-i18n branch.
Now that the sprint-i18n branch has two different English language locales (en_US and en_CA), it surfaces the fact that they both show up as "English" in the dropdown.

Given this, if you don't mind, I would like:

  1. to have this PR targeted at the sprint-i18n branch, and
  2. to update the PR to include both the language and locale.

Thoughts?

@gneissone
Copy link
Member Author

What do you suggest is the default behavior? Always show place name?

'English (United States)'
'English (Canada)'
'Français (Canada)' ?

Or do we want the place only if there are multiple of the same? Will sites want to show multiple versions of the same language?

@gneissone gneissone changed the base branch from master to sprint-i18n May 22, 2020 22:56
@gneissone gneissone changed the base branch from sprint-i18n to master May 22, 2020 23:01
@gneissone gneissone changed the base branch from master to sprint-i18n May 22, 2020 23:01
@gneissone
Copy link
Member Author

Based on comment on Slack, updated to include country name.
Screen Shot 2020-05-22 at 4 51 38 PM

Copy link
Member

@awoods awoods left a comment

Choose a reason for hiding this comment

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

I was unable to see the changes introduced here because this PR branch is not current with the sprint-i18n branch. Specifically, the versions in the pom.xml files all indicate "1.11.1-SNAPSHOT" instead of "1.11.2-SNAPSHOT"... which is not picked up by VIVO during build.

@gneissone
Copy link
Member Author

I was unable to see the changes introduced here because this PR branch is not current with the sprint-i18n branch. Specifically, the versions in the pom.xml files all indicate "1.11.1-SNAPSHOT" instead of "1.11.2-SNAPSHOT"... which is not picked up by VIVO during build.

An unfortunate side-effect of jumping between pointing the pull request at master and i18n-sprint. It will be nice to get master merged into the sprint branch. Out of curiosity, @awoods did you check out my branch or are you checking out the pull request via git fetch origin pull/ID/head:BRANCHNAME? I wonder if the latter method avoids this issue.

@awoods
Copy link
Member

awoods commented May 26, 2020

I checked out the pull/ID

Also noting the the version number in the master branch is also "1.11.2-SNAPSHOT".

@gneissone
Copy link
Member Author

Right, I just rebased my branch back in time to a point before there were merge conflicts between master and sprint-i18n, saw there were only relevant files in my diff on the pull request and called it a day. I should have merged the current sprint-i18n branch back in, which I have now done.

@gneissone gneissone requested a review from awoods May 26, 2020 18:07
@awoods
Copy link
Member

awoods commented May 27, 2020

This looks good... and we can move forward as it stands. I will note, however, that the dropdown items would render better if the width were expanded.
See:
Screenshot from 2020-05-27 12-52-18

@gneissone , thoughts?

@gneissone
Copy link
Member Author

I already pushed out the min-width by 40 pixels so it looked good on my screen, didn't try other resolutions. I wonder if it's necessary to set the width at all, or if it can be as wide as the longest list item.

@awoods
Copy link
Member

awoods commented May 27, 2020

I already pushed out the min-width by 40 pixels so it looked good on my screen, didn't try other resolutions. I wonder if it's necessary to set the width at all, or if it can be as wide as the longest list item.

If removing the width allows for dynamically appropriate sizing... that would be great.

@gneissone
Copy link
Member Author

@awoods Changed the min-width to be an em value, see if that works for you. If not, please suggest a width that looks okay.

@gneissone gneissone requested a review from awoods May 29, 2020 18:42
@nicalico
Copy link
Contributor

nicalico commented Jun 9, 2020

Great improvement! However, capitalizing is not consistent. I suggest to change

<a href="${selectLocale.selectLocaleUrl}?selection=${locale.code}" title="${i18n().select_locale} -- ${locale.label}">${locale.label}<#if locale.country?has_content> (${locale.country})</#if></a>

to

<a href="${selectLocale.selectLocaleUrl}?selection=${locale.code}" title="${i18n().select_locale} -- ${locale.label}">${locale.label?capitalize}<#if locale.country?has_content> (${locale.country})</#if></a>

@gneissone
Copy link
Member Author

@nicalico I incorporated your capitalization suggestion, thanks. I don't have a nice cross-browser solution for the dropdown width, but I'm open to suggestions or they could be submitted as part of a different pull request.

@gneissone gneissone requested a review from awoods June 15, 2020 18:48
@nicalico
Copy link
Contributor

Thanks for the capitalizing. As for the Firefox fix, I have little time to work on it at the moment -- not to mention my css-fu isn't outstanding -- so maybe we should just move the PR forward as suggested by @awoods, and reopen the ticket when a fix occur to us.

@nicalico
Copy link
Contributor

@gneissone I have a hard time untangling the commits, here. Did you change something to the css? It doesn't look so, and yet the menu now displays correctly in FF.

@gneissone
Copy link
Member Author

@nicalico Perhaps its easier to just look at the file diff instead of the commit chain, since adjustments were made to the css multiple times based on feedback.

I didn't change anything in the css in the last commit. I changed the dropdown to be slightly wider on May 29, though.

@nicalico
Copy link
Contributor

@gneissone Yes, I saw that -- but I reported the FF bug on June 6th. Beats me. At any rate, I pulled / recompiled 30 minutes ago, and it looks perfect. Good to merge, in my opinion.

@awoods awoods merged commit 289b5b2 into sprint-i18n Jun 22, 2020
@awoods awoods deleted the hotfix/VIVO-1839 branch June 22, 2020 14:22
roflinn added a commit that referenced this pull request Feb 10, 2021
* Layer uqam updates onto master (minus trailing whitespace)

* Update RDFServiceFactorySingle.java

* Sprint i18n whitespace (#143)

* Removed extraneous whitespace
   - modified:   api/src/main/java/edu/cornell/mannlib/vitro/webapp/edit/n3editing/VTwo/BaseEditElementVTwo.java

* move RootUserPolicy.java from VIVO to Vitro repo.

* Remove blank line

* Fixed indentations

* Update whitespace for: TemplateProcessingHelper.java

* Update whitespace for: RDFServiceModel.java

* Removed extraneous whitespaces.

* Tagging UQAM Comments with following tags

-Add-Feature
-Optimization
-Linguistic-Management
-Bug-Correction

* Resolve compilation failures in Vitro tests

* Remove whitespace changes from SelectListGeneratorVTwo.java

* Add null check on field values of RDF Form (#158)

Resolves: https://jira.lyrasis.org/browse/VIVO-1800

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* Fix incorrect tool-tip in language selection dropdown (#156)

Resolves: https://jira.lyrasis.org/browse/VIVO-1783

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* Sprint i18n/lang filtering model (#159)

* Add language-aware graph filtering instead of layering on additional RDFService

Resolves: https://jira.lyrasis.org/browse/VIVO-1771

* Remove hardcoded link label (#161)

Resolves: https://jira.lyrasis.org/browse/VIVO-1779

* Enable filtering of non-enabled i18n language files from being loaded into triple store (#160)

Define name of available language listing file (found in Vitro-languages)
Update RDFFilesLoader method interfaces to include ServletContext

Part of resolution to: https://jira.lyrasis.org/browse/VIVO-1836

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* Add i18n version of Edit page title (#157)

Related to: https://jira.lyrasis.org/browse/VIVO-1779

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* Changing ontology extensions for n3 & assigning a base IRI to each (#163)

ontology

Co-authored-by: michelheon <heon@videotorn.ca>
Co-authored-by: Andrew Woods <awoods@duraspace.org>

https://jira.lyrasis.org/browse/VIVO-1862

* Update sprint-i18n with master branch (#166)

* Make data property richtext editor option selectable from UI

* [VIVO-1755] - Better error handling when reasoner disabled (#137)

* Better error handling when reasoner disabled

* Change reasoner error log message to debug

* Extend reasoner status error handling

* Improve reasoner error log message and extend to JenaAdminActions

* Bump up log level for admin action to 'warn' and edit admin panel error message

* Change Model writer lang from "N3-PP" to "N3" (#149)

Resolves: https://jira.lyrasis.org/browse/VIVO-1761

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* [VIVO-1851] - Add 'remove' option to named graph management page (#162)

* Add 'remove' option to named graph management page

Resolves: https://jira.lyrasis.org/browse/VIVO-1851

* [VIVO-1872] - Add download option to SPARQL Query page (#164)

* Add download option to SPARQL Query page

* Set SPARQL query results content type even if downloading

* Address pull request comments

Co-authored-by: gneissone <mbgross@wustl.edu>
Co-authored-by: Ralph O'Flinn <roflinn@users.noreply.github.com>
Co-authored-by: Andrew Woods <awoods@duraspace.org>

* [VIVO-1839] - Use language name for selector instead of flags (#165)

* Use language name for selector instead of flags
* Add country name to language selector
* Increase min-width of language dropdown
* Update data tree comment in languageSelector.ftl
* Min-width for language dropdown
* Language dropdown css adjustment
* Capitalize locale label
* Indent on languageSelector.ftl

Resolves: https://jira.lyrasis.org/browse/VIVO-1839

* Adds internationalization to the admin/sparql-query page (#167)

* Adds internationalization to the admin/sparql-query page

Partial resolution of: https://jira.lyrasis.org/browse/VIVO-1873

* Add i18n for 'save query result'

Related to: https://jira.lyrasis.org/browse/VIVO-1873

* code review

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* removed redundat files which are now available via Vitro- and VIVO-languages

* Making UQAM-optimization aware of https://

* Add parent reference to installer/pom.xml (#174)

Resolves: https://jira.lyrasis.org/browse/VIVO-1903

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* Revert non-function RDF changes (#177)

Related to: https://jira.lyrasis.org/browse/VIVO-1905

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* Ensure 'other' in Individual->Teaching->advisees->type is translated

Partial resolution to: https://jira.lyrasis.org/browse/VIVO-1881

* Re-add logic from commit 7420957

* [VIVO-1812] use i18n translation for validation messages (#179)

* replace constants with i18n text in BasicValidationVTwo
* use constants for i18n template key

Partial resolution of: https://jira.lyrasis.org/browse/VIVO-1812

* added js_string at i18n strings to handle quotes properly, ticket vivo-1842 (#180)

Partial resolution for: https://jira.lyrasis.org/browse/VIVO-1842

* [VIVO-1870] simplify and concat translation for browse by vclass (#182)

* simplify and concat translation for browse by vclass

Partial resolution of: https://jira.lyrasis.org/browse/VIVO-1870

* [VIVO-1900] i18n: added empty check for getCountry, fixing bug with spanish label (es) (#181)

* added empty check for getCountry, fixing bug with spanish label (es), ticket vivo-1900

Partial resolution to: https://jira.lyrasis.org/browse/VIVO-1900

* [VIVO 1870] update i18n key to JavaScript mapping (#183)

* update menupage-script i18n key to JavaScript mapping

Follow-on to resolution of: https://jira.lyrasis.org/browse/VIVO-1870

* [VIVO-1837] get i18n bundle from WebappDaoFactoryConfig preferred locales (#178)

* apply i18n text to VClassDaoJena.getLabelForClass
* expose i18n bundle through webapp dao factory interface
* use whole messages for i18n
* if request available use request bundle else use context bundle
* clear cache when context theme directory changes
* make getOverridingLocale from context private
* select locale based on preferred locale from webapp dao factory
* use first preferred local even when no selectable locales available on context
* build default preferred locales from default preferred languages

Resolves: https://jira.lyrasis.org/browse/VIVO-1837

* Ensure that "available langs" include base langs (#185)

Resolves: https://jira.lyrasis.org/browse/VIVO-1922

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* Ensure I18nStub is initialized before pertinent tests (#186)

Resolves: https://jira.lyrasis.org/browse/VIVO-1923

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* add additional null checks to ensure context is ready for theme change (#187)

* [VIVO-1915] i18n - Adding a label with language tag through the "new" manageLabelsForindividualAddForm.ftl does not take the setting for language tag (#184)

* Fixed bug when addng a label with language tag through managLabelsForindividualAddForm, re ticket VIVO-1915

* removed the language select for managing multi language labels, ticket vivo-1915

* readded the fix for language selection because of the fr-CA version of the manageLabelsForIndividualftl, ticket vivo-1915

* removed earlier fix, removed comments, modified ManageLabelGenrator, ticket vivo-1915

Partial resolution to: https://jira.lyrasis.org/browse/VIVO-1915

* - replaced hard coded "or"s with i18n().or

* [VIVO-1925] i18n: Editing labels results in new label (#188)

* fixed bug: Editing language labels results in new label, ticket vivo-1925

Resolves: https://jira.lyrasis.org/browse/VIVO-1925

* Enable lang selection without need for 'available-langs.properties' (#169)

* Enable lang selection without need for 'available-langs.properties'
Co-authored-by: Andrew Woods <awoods@duraspace.org>

* Reduce number of times QuerySolutions are looped in LanguageFilteringRDFService (#194)

- No functional change in this update

Resolves: https://jira.lyrasis.org/browse/VIVO-1931

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* Prevent 'ProcessRdfForm.parseN3ToRDF' from using null linguisticContext (#197)

Resolves: https://jira.lyrasis.org/browse/VIVO-1944

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* [Vivo 1918] i18n: Reload firsttime files on start-up if changed (#195)

Co-authored-by: Andrew Woods <awoods@lyrasis.org>

* [VIVO-1936] i18n: updated the language comment in runtime.properties

* ftl function to capitalize group name affording override

* Further i18n for BasicValidationVTwo.java

* Fixed DeletePropertyController.java - getting localname of property properly (#200)

* Fixed DeletePropertyController.java - getting localname of property properly

Resolves: https://jira.lyrasis.org/browse/VIVO-1816

* Removed hardcoded time units by properties calls in dateTimeWithPrecision.ftl

* Use i18n values for date time form (#204)

Resolves: https://jira.lyrasis.org/browse/VIVO-1953

* VIVO-1929: patch authorizing create individual form (#206)

* patch authorizing create individual form

Resolves: https://jira.lyrasis.org/browse/VIVO-1929

* Issue/vivo 1947 (#205)

* Make data property richtext editor option selectable from UI
* Better error handling when reasoner disabled
* Change reasoner error log message to debug
* Extend reasoner status error handling
* Improve reasoner error log message and extend to JenaAdminActions
* Bump up log level for admin action to 'warn' and edit admin panel error message
* Change Model writer lang from "N3-PP" to "N3" (#149)
* Prevent ontology editor and N3 editing from deleting property values in languages other than the one associated with the editing request. [https://jira.lyrasis.org/browse/VIVO-1947]

Resolves: https://jira.lyrasis.org/browse/VIVO-1947

Co-authored-by: gneissone <mbgross@wustl.edu>
Co-authored-by: Ralph O'Flinn <roflinn@users.noreply.github.com>
Co-authored-by: Andrew Woods <awoods@lyrasis.org>
Co-authored-by: Andrew Woods <awoods@duraspace.org>

* Remove comments

Co-authored-by: Andrew Woods <awoods@duraspace.org>
Co-authored-by: Nicolas D <46490666+nicalico@users.noreply.github.com>
Co-authored-by: VIVO UQAM <62542918+UQAM-VIVO@users.noreply.github.com>
Co-authored-by: Matthias Luehr <luehr@hs-mittweida.de>
Co-authored-by: michelheonuqam <heon.michel@uqam.ca>
Co-authored-by: Brian Lowe <brianjlowe@gmail.com>
Co-authored-by: j-dornbusch <joachim.dornbusch@ehess.fr>
Co-authored-by: Michel Heon <heon@videotron.ca>
Co-authored-by: gneissone <mbgross@wustl.edu>
Co-authored-by: Ralph O'Flinn <roflinn@users.noreply.github.com>
Co-authored-by: dofeldsc <dofeldsc@uos.de>
Co-authored-by: root <root@vivo-development.hs-mittweida.de>
Co-authored-by: gneissone <mbgross@unavco.org>
Co-authored-by: William Welling <wwelling@library.tamu.edu>
Co-authored-by: Kampe <Benjamin.Kampe@tib.eu>
Co-authored-by: Gross, Benjamin <benjamin.gross@clarivate.com>
Co-authored-by: matthiasluehr <60263380+matthiasluehr@users.noreply.github.com>
Co-authored-by: nicolasdickner <dickner.nicolas@uqam.ca>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants