Skip to content
This repository has been archived by the owner on May 3, 2022. It is now read-only.

EZP-23765: As a PlatformUI user I want to view different language ver… #251

Merged
merged 1 commit into from Jul 1, 2015
Merged

EZP-23765: As a PlatformUI user I want to view different language ver… #251

merged 1 commit into from Jul 1, 2015

Conversation

mhyndle
Copy link
Contributor

@mhyndle mhyndle commented Jun 10, 2015

JIRA: https://jira.ez.no/browse/EZP-23765

Description

The goal of the story is to allow user switch viewed language of content by using dropdown list with existing translations. After choosing other translation app should navigate to the same location view but with selected language parameter. There will be language indicator in upper right corner of "Content" section.

Screencast: https://youtu.be/MYP8XpRlSZc

Tasks

  • Implementation
  • CSS
  • Tests
  • Rebase

Related issues:

@@ -1,4 +1,5 @@
<h2 class="ez-raw-content-title"><a href="#">Content</a></h2>
<div class="ez-language-switcher"></div>
Copy link
Contributor

Choose a reason for hiding this comment

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

usually we add the -container on such element so it should be called ez-language-switcher-container.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's also an inconsistency here, the language switcher is a sub view of the LocationView but its container is added to the RawContentView template. It's either a sub view of the LocationView and its container is added to the LocationView template or a sub view of the RawContentView and it's ok to add the container in the rawcontent.hbt template.

@mhyndle
Copy link
Contributor Author

mhyndle commented Jun 19, 2015

Ready for review
ping @dpobel @yannickroger @StephaneDiot @andrerom

* For full copyright and license information view LICENSE file distributed with this source code.
*/

.ez-language-switcher-container {
Copy link
Contributor

Choose a reason for hiding this comment

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

the empty rules can be removed

@dpobel
Copy link
Contributor

dpobel commented Jun 25, 2015

Nice work :)
Besides the inline comments, I've noticed a few issues:

  • The flag of the current translation and the flags in the list are not aligned flags. I would also make the shadow a bit darker (btw, in a review for new UI thing a screencast or at least a screenshot is always welcome)

  • A CSS transition when the list (dis)appears would be nice. Since the list is absolutely positioned and appears like a vertical list, a transition on a scaleY transform seems appropriate and quite easy to do. So basically, in the theme stylesheet you should set the list with display: block but hide it with the transform: scaleY(0), then when it's supposed to be displayed you have to set transform: scaleY(1) and to a get a nice transition

      -webkit-transition: all 0.3s ease;
              transition: all 0.3s ease;

    would do the trick.

  • The getter for eZ.Content currentVersion attribute is not really tested. You should check that it returns an instance of eZ.Version with its attributes consistent with the raw value of the currentVersion attribute.

  • eZ.Version.getTranslationsList is not tested

  • the RawContentView instantiated in LocationViewView now receives some new parameters, this should be tested

  • The ez-getflagcodehelper is not tested at all.

@mhyndle
Copy link
Contributor Author

mhyndle commented Jun 26, 2015

I think I haven't miss anything from @dpobel 's comments, so please review it again (I will rebase commits after positive review). Added the screencast which is presenting the behaviour of the language switcher, however navigating to translations is not working as it's waiting for this one: https://jira.ez.no/browse/EZP-24384. However, there shouldn't be need to fix anything after that in language switcher as path helper is already called with languageCode parameter.

display: block;
transform: scaleY(0);
transform-origin: 50% 0%;
-webkit-transform-origin: 50% 0%;
Copy link
Contributor

Choose a reason for hiding this comment

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

please order, group and align the rules like (and -webkit-transform is missing and -webkit-transition is not needed):

        transform: scaleY(0);
-webkit-transform: scaleY(0);
        transform-origin: 50% 0%;
-webkit-transform-origin: 50% 0%;
transition: all 0.3s ease;

Copy link
Contributor

Choose a reason for hiding this comment

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

after testing it, 0.3s is maybe a bit slow in this case, I think 0.2s would give a better result.

@dpobel
Copy link
Contributor

dpobel commented Jun 29, 2015

almost there! One last nitpick on the translation list CSS, again on the alignement but this time for the small arrow:

flags

Besides that and the last inline comments, +1

@crevillo
Copy link
Contributor

to flag or not to flag, that'ts the question... :)

…sions of a content object

Conflicts:
	Resources/config/css.yml
@mhyndle
Copy link
Contributor Author

mhyndle commented Jun 29, 2015

Rebased
ping @dpobel @yannickroger @StephaneDiot @andrerom

@dpobel
Copy link
Contributor

dpobel commented Jun 30, 2015

+1

dpobel added a commit that referenced this pull request Jul 1, 2015
…e_versions_of_content_object

EZP-23765: As a PlatformUI user I want to view different language ver…
@dpobel dpobel merged commit e594ad2 into ezsystems:master Jul 1, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
3 participants