Umami demo has display problems in right-to-left languages

Regarding point 2

The front page recipe content is missing (and the "No front page content has been created yet" message is displayed instead).

This main block displaying above message is actually a view http://192.168.33.10/d86/en/admin/structure/views/view/frontpage which has got a 'Translation language' filter. So when we switch to another language (irrespective of left to right or right to left language), there is no content already created for that language, which can appear in this view, we need to create new content for new language to make them appear in view.

The second only happened for me if I switched languages after installing in English (i.e., by manually enabling the core Language module and then going to admin/config/regional/language to add a new language and switch to it); if I installed from scratch in Hebrew, the front page content was shown correctly.

Yes, this is true, because when the site is installed from scratch in one language and language module is not enabled , this 'Translation language' filter in view doesn't come into thepicture.

In short this problem is not related to umami profile.

I agree with @cchoudhary in #2 that we shouldn't worry about the situation where Umami is installed in US English, then Language module enabled and the language switch until we support multi site.

However, we should not look obviously broken when installing after selecting a right to left language such as Hebrew on the first page of the installer.

I've just tested briefly with the latest version of Umami from Drupal 8.6.1 and this issue is not nearly as bad as it was when @David_Rothstein took the screenshots above. Therefore suggest next steps for this are to install in Hebrew and take screenshots of the different pages side by side with the default US English version, and highlighting where we need fixes. This would be a great novice task for Drupal Europe.

Note to install in Hebrew, select the following option on the first screen of the Drupal installation wizard (towards the bottom of the list):

This is being worked on by Gvert at DrupalEurope who I am mentoring.

Robindh and I will be working on this at Drupal Europe.

1. The links styling on the login page isn't correct in the right-to-left version, there is some spacing visible between the links
   
   English (ltr):
   
   
   Hebrew (rtl):
   
   
2. On a recipe page, the arrows between the breadcrumbs aren't correclty in the right-to-left version, they point into different directions
   
   English (ltr):
   
   
   Hebrew (rtl):
   
   
3. On the recipe page, the icons next to the additional information aren't aligned correctly in the right-to-left version.
   
   English (ltr):
   
   
   Hebrew (rtl):
   
   
4. On the recipe page, the numbering next to the instruction steps isn't correctly aligned in the right-to-left version.
   
   English (ltr):
   
   
   Hebrew (rtl):
   
   
5. On the search result page, the styling of the input field for a new search is broken in the right-to-left version.
   
   English (ltr):
   
   
   Hebrew (rtl):
   
   
6. On the search result page, the title "Search results" isn't correctly aligned in the right-to-left version.
   
   English (ltr):
   
   
   Hebrew (rtl):
   
   

1. The alert message icon is floated to the left of the page, it should be aligned to the right with the text.

English (ltr):

Hebrew (rtl):

2. On the contact page, at the bottom of the form: submit buttons do not have the correct padding / margins.

English (ltr):

Hebrew (rtl):

3. Margins on containers should be inverted:

• .view-content should have "margin-left: 14px" instead of "margin-right: 14px"
• .attachment-after should have "margin-right: 14px" instead of "margin-left: 14px"

English (ltr):

Hebrew (rtl):

4. Should the "find out more" arrow in the footer point the other way?

English (ltr):

Hebrew (rtl):

5. The search box styling in the header should be adapted for right to left and the "home" link should be correctly aligned with the other menu items. The "login" link in the header should be aligned left.

English (ltr):

Hebrew (rtl):

6. The "logout" and "user" links in the header should be aligned left

English (ltr):

Hebrew (rtl):

Restore Gvert's files, since I accidentally deleted them all.

Nice work by @Gvert and @robindh on the screenshots! Happy to say I have a fix for all of the issues but with some minor things not addressed (requiring some work)

• The search form in the header has a 1 or 2 pixel misalignment at the bottom; seems like the cause is the difference in fonts
• The search result page form is not responsive and the alignment breaks on smaller screens because of a mix of fixed width and flex box properties.

Would it be possible to get screenshots of these fixes attached to this issue? I think we are going to need them to keep track of what is fixed by the issue.

Setting back to 'Needs work' so someone can gather the screenshots.

I haven't installed the site with this patch yet, but reading through it, it looks very promising, so thanks for all the work on it so far.

Here are the screenshots

Header

• Search block - Alignment and border issues
• Account menu - Links alignment
• Main menu - Spacing issue

Login Page

• Login page tab link spacing
  

Recipe Page

• Breadcrumb arrow directions
  
• Recipe page - List Icons
  
• Recipe page - List numbering
  

Search Results Page

• Exposed Filter
• Heading and listing alignment

Home Page

• Promoted Content Spacing

Contact Page

• form buttons

Misc

• Alert message icon alignment
  
• Footer - "Find out more" arrow
  

This is a huge improvement over what we currently have - even though we are not officially supporting RTL. It's great that even though we don't officially support RTL, things now don't look broken in RTL.

Let's mark this as RTBC, and if we find any other small RTL bugs later, we can squash them easily.

Thanks a lot for the work on this, especially @Gvert and @tsega and @robindh. Please continue to help us make the Out of the Box experience even better!

Well done @Gvert and @Robindh for working on this looking great!

@markconray, I'm glad to be of help! You can count me in on contributing to Umami or any other Drupal project for that matter. Participating in the mentored contributions was an eye-opener for me, thanks @ELI-T and @cferthorney for your kindness and patience in explaining the whole process.

On to the next then.

We have some coding standards for comments in .css files for stuff that needs RTL specific styling. I think we should apply them here. See https://www.drupal.org/docs/develop/standards/css/css-formatting-guideli...

+++ b/core/profiles/demo_umami/themes/umami/css/components/blocks/search/search.css
@@ -6,6 +6,9 @@
 .search-form + h2 {
   margin: 0 14px 1rem;
 }
+[dir=rtl] .search-form + h2 {
+  margin: 0 1rem 1rem;
+}

So for example we need to add /* LTR */ to the LTR version here. This is so that if future us makes a change to the LTR version we know there is an RTL version to update.

Added a patch to add the /* LTR */ comments for the CSS codes which have a RTL version.

Yay, thanks for working on this!

+++ b/core/profiles/demo_umami/themes/umami/css/components/navigation/tabs/tabs.css
@@ -13,9 +13,12 @@
 .tabs .tab {
-  margin: 0;
+  margin: 0; /* LTR */
   background-color: #e6eee0;
 }
+[dir=rtl] .tabs .tab{
+  margin: 0;
+}

An all around 0 margin is not surprisingly not different in RTL either and therefore should not be marked as LTR and not have an RTL counterpart with the same style :)

(Also this fixes a missing space in the sheet :)

Went ahead and reviewed the rest, sorry for not posting altogether.

1. +++ b/core/profiles/demo_umami/themes/umami/css/components/blocks/search/search.css
   @@ -4,7 +4,10 @@
   -  margin: 0 14px 1rem;
   +  margin: 0 14px 1rem; /* LTR */
   +}
   +[dir=rtl] .search-form + h2 {
   +  margin: 0 1rem 1rem;
   

   How did 14px became 1rem in the RTL version?

2. +++ b/core/profiles/demo_umami/themes/umami/css/components/blocks/search/search.css
   @@ -110,6 +131,13 @@
   -  border-top-left-radius: 4px;
   -  border-bottom-left-radius: 4px;
   +  border-top-left-radius: 4px; /* LTR */
   +  border-bottom-left-radius: 4px; /* LTR */
   +}
   +[dir=rtl] .search-block-form .form-submit:focus,
   +[dir=rtl] .search-block-form .form-submit:hover,
   +[dir=rtl] .search-form .form-submit:focus,
   +[dir=rtl] .search-form .form-submit:hover {
   +  margin: 0;
   +  border-radius: 4px;
    }
   

   Why is RTL getting a full border radius while LTR only has selective?

3. +++ b/core/profiles/demo_umami/themes/umami/css/components/messages/messages.css
   @@ -17,7 +17,10 @@
    .messages__content {
   -  background: no-repeat 0 center;
   +  background: no-repeat 0 center; /* LTR */
   +}
   +[dir=rtl] .messages__content {
   +  background-position: right;
    }
   

   Hm, if this was normally in the center, why does it go to the right then?

4. +++ b/core/profiles/demo_umami/themes/umami/css/components/navigation/breadcrumbs/breadcrumbs.css
   @@ -11,4 +11,7 @@
   +  [dir=rtl] .breadcrumb li {
   +    display: inline-block;
   +  }
   

   Why was this not applied to LTR? Does it harm something there? What's the benefit in RTL?

tagging for badcamp 2018

Thanks for the review guys! @imalabya nice work on the improvements!

@gábor:

1. An all around 0 margin is not surprisingly not different in RTL either and therefore should not be marked as LTR and not have an RTL counterpart with the same style :)

The addition of the margin: 0; in RTL was because the base theme (Classy) was adding margins to tabs specifically for RTL and hence the need override that in Umami.

core/themes/classy/css/components/tabs.css:18

[dir="rtl"] .tabs > li {
  margin-left: 0.3em;
  margin-right: 0;
}

2. How did 14px became 1rem in the RTL version?

That was changed to keep the Search Result title and result item boxes aligned on the right. See below:

3. Why is RTL getting a full border radius while LTR only has selective?

As you can see form the LTR hover/focus state of the search button, all corners have a radius. I believe that is inherited from core/profiles/demo_umami/themes/umami/css/base.css:65. The reason that I'm applying the radius again is because for RTL we also have to switch the corners that are square and rounder; meaning a more specific styling that overrides the default one in base.css. Then on hover, the RTL specific one needs to be overridden to apply border radius for all corners. :D

Hm, if this was normally in the center, why does it go to the right then?

The RTL rule is actually overriding the 0 part (equivalent to left background-position: left;) in the LTR. The center is for the vertical position which is shared by both.

Why was this not applied to LTR? Does it harm something there? What's the benefit in RTL?

For the most part I worked on the RTL version of things, totally ignoring LTR. In this case, in display: inline-block; is needed to fix the "swapping" issue on pages that have a third-level breadcrumb. See below:

It would be nice to add non-directional changes to the LTR sheet (the inline-block thing), so that the RTL is limited to directional changes rather than layout changes.

For the margin question, if the LTR one was margin: 0 14px 1rem; then both left and right are 14px, no? Why does that need to change for both left and right to 1rem in RTL? Sounds like 1rem would have been better originally, is that not aligned with other things in LTR? Then there is a sizing inconsistency between the two things placed?

@Gábor, I have looked into the breadcrumb issue and applying the display: inline-block; style to both LTR and RTL produces the expected result and is consistent with the design.

Similarly, I have checked the 14px -> 1remand it seems applying the 1rem value to the original style does not affect the design significantly (noticeably even); it only add 2 pixels more to the search results Header in LTR which is not aligned to anything else.

Yay thanks, looks good other than this small thing:

+++ b/core/profiles/demo_umami/themes/umami/css/components/blocks/search/search.css
@@ -4,10 +4,7 @@
+  margin: 0 1rem 1rem; /* LTR */

hm if its the same value left/right, we don't usually add LTR since it would not need an RTL counterpart

Thanks for looking into this Gábor and nice catch!

I've made the changes.

Added #2984447: Feature request: add multilingual functionalities to umami demo as parent.

Great work everyone, thanks for all the help here. This is a great improvement.

Assigning credits.

While trying to commit these two style issues were identified by stylelint:

core/profiles/demo_umami/themes/umami/css/components/forms/contact.css
 46:57  ✖  Expected single space before "{"   block-opening-brace-space-before

core/profiles/demo_umami/themes/umami/css/components/navigation/tabs/tabs.css
 19:20  ✖  Expected single space before "{"   block-opening-brace-space-before

@Gábor, here are the fix for the above.

Back to RTBC

+++ b/core/profiles/demo_umami/themes/umami/css/components/blocks/search/search-results.css
@@ -96,7 +98,10 @@
+  margin: 0 0 1rem 0; /* LTR */
...
+  margin: 0 1rem 1rem 0;

+++ b/core/profiles/demo_umami/themes/umami/css/components/blocks/search/search.css
@@ -110,6 +128,13 @@
   margin: 0;
-  border-top-left-radius: 4px;
-  border-bottom-left-radius: 4px;
+  border-top-left-radius: 4px; /* LTR */
+  border-bottom-left-radius: 4px; /* LTR */
...
+  margin: 0;
+  border-radius: 4px;

I think for these non-obvious not opposite LTR to RTL changes we should have a comment. So people don't file issues in the future saying this is wrong because the RTL version is not the opposite of the LTR version.

@alexpott I have added comments to the changes you've highlighted. Hope they are helpful enough to avoid future issues.

Third time lucky - back to RTBC

Yay, thanks, committed!