Unclear what is compared with the visual plugin

should we add the revision links next to the "Comparing two revisions" text?

Not really sure where to place the revision links.

ohhh fyi the navigation label appears in the screenshot because there was another link in the right of the screen, I checked the code and the navigation label doesn't appear when there are no more than two revisions.

Edited weight typos and added values on the missing points (ex. diff display).
Currently weight for the "Comparing" div is 0 and for "Navigation" 1. They should swap but they don't. If "Comparing" weight is set to -1 then they swap the places. Screenshot included.

As discussed remove that extra wrapper for navigation, and things will work magically. :)

Done, it is working properly now

Comment 10 same as comment 8. I uploaded wrong patch.
Here it is now, with proper patch and working.

+++ b/src/Plugin/diff/Layout/VisualInlineDiffLayout.php
@@ -154,7 +154,18 @@ class VisualInlineDiffLayout extends DiffLayoutBase {
-
+    $build['diff_revisions'] = [
+      '#type' => 'item',
+      '#title' => $this->t('Comparing:'),
+      '#weight' => 0,
+    ];
+    $build['diff_revisions'][] = [
+      '#markup' => $this->buildRevisionLink($left_revision),
+      '#suffix' => '<br>',
+    ];
+    $build['diff_revisions'][] = [
+      '#markup' => $this->buildRevisionLink($right_revision),
+    ];

Please add a white line before and after this. If you can add a comment like: "Show the revisions that are compared." is also good.

+++ b/src/Plugin/diff/Layout/VisualInlineDiffLayout.php
@@ -154,6 +154,19 @@ class VisualInlineDiffLayout extends DiffLayoutBase {
+    // Show the revisions that are compared

"." missing at the end of the comment. Please also add a white line before this line.

Don't forget to set it as needs review.

Looks nice. Not really sure if the links are fine one below the other. Besides that I would say RTBC.

Tested this locally and I've noticed that if a revision is made in few seconds, the time displayed in the link is the same, thus it could confuse the user, but the revision log is also shown there (maybe needs a bit more work).

Usually this kind of page should not display too many info, but I guess it would be better to somehow show the revision log and the "date - time" (as a while link (?)) (?)

PS: @drobnjak, please remember to provide a new screenshot when you have some new UI updates ;)

+++ b/diff.install
@@ -53,19 +53,8 @@ function diff_update_8004() {
-  // The original update function had a bug, it set the new configuration of
-  // layout plugins using the old keys (which were changed). Update 8006 will
-  // fix this issue.
-}
-
-/**
- * Update settings, set the new keys of layout plugins.
- */
-function diff_update_8006() {
   $config = \Drupal::configFactory()->getEditable('diff.settings');
-  $layout_manager = \Drupal::service('plugin.manager.diff.layout');
-  $layout_manager->clearCachedDefinitions();
-  $plugins = $layout_manager->getDefinitions();
+  $plugins = \Drupal::service('plugin.manager.diff.layout')->getDefinitions();

I guess you didn't update your branch before :P

Otherwise looks really nice! :)

That is an API change, not sure if we can make it now?

Didn't want to change the IS.

I was wondering if in the end we should remove the whole revision log since it is used also in the header. That info shouldn't go there. Not sure, would like to know Miro's feedback.

In that case would be better IMHO to place all the navigation things in the outside-in tray :)

Removed semi-colon and uploading new patch as @miro_dietiker suggested.

I added author in the middle between revision date and message since the revision message is much longer than the author name in some cases. They are concatenated now. Implementing flex or columns would be time consuming and since the current navigation will probably move into the sidebar after this issue https://www.drupal.org/node/2808623 , I think it is better to devote time to investigate outside-in possibility to implement it with the diff module before implementing flex or column layout here for the navigation.

As @miro_dietiker suggested, I implemented flex layout. I also changed author to be a link and did the css adjustments to support different resolutions, tablet and smartphone screens.

Test fix

1. +++ b/src/DiffLayoutBase.php
   @@ -112,14 +113,34 @@ abstract class DiffLayoutBase extends PluginBase implements DiffLayoutInterface,
   +      ¶
   

   Wrong indentation.

2. +++ b/src/Form/RevisionOverviewForm.php
   @@ -244,7 +244,7 @@ class RevisionOverviewForm extends FormBase {
   +                  Url::fromRoute('c', ['node' => $node->id(), 'node_revision' => $vid, 'langcode' => $langcode]) :
   

   Why are you changing the route?

3. +++ b/src/Tests/DiffRevisionTest.php
   @@ -95,7 +95,8 @@ class DiffRevisionTest extends DiffTestBase {
   -    $this->assertEqual((string) $comment, 'Revision 2 comment');
   +    $this->assertEqual((string) $comment, '
   +        ');
   

   Why this change is needed?

Responding to comment #41
1. Indentation fixed
2. Route change happened accidentally while searching through code and it went unseen in the commit
3. When DiffLayoutBase changes were introduced $comment moved position and its value is now "\n " which was changed in the test

New patch uploaded with fixes for the tests

Hmmm just tested manually, with your changes the header of the Split/Unified fields layout is broken, it basically adds too much space when a revision has a log and the other doesn't. We better need to remove the revision log when using that function on the header of those layouts. I'm not really sure how to fix this, there might be lots of ways. but for sure we cannot allow have the table headers like they are now.

+++ b/src/DiffLayoutBase.php
@@ -112,14 +112,34 @@ abstract class DiffLayoutBase extends PluginBase implements DiffLayoutInterface,
+      $revision_link = '
+        <div class="comparation-flex-container">
+          <div class="comparation-flex-item-date">';
+            $revision_link.=$revision_date_url;
+            $revision_link.='</div>
+          <div class="comparation-flex-item-author">';
+            $revision_link.=$revision_author_url;
+            $revision_link.='</div>
+          <div class="comparation-flex-item-message">';
+            $revision_link.=$revision_log;
+            $revision_link.='</div>
+        </div>';

btw this cannot be done in this way, better if you change it to type => item and create the elements inside.

Changed the code, this time using type -> item

This fixes your problem of array, not really sure if the changes are according with what you wanted to do. please check the markup.

Split revision log creation for visual inline from split fields and unified fields since it was breaking the header and DiffRevisionTest. Changed previous_revision and revision to left and right as it is used in the code.
Uploading a patch now for tests, and if it passes tests and reviews it would be ready to commit. I suggest creating follow up for the 49.1. and access to user profiles.

Small test fix

1. +++ b/src/DiffLayoutBase.php
   @@ -118,7 +177,7 @@ abstract class DiffLayoutBase extends PluginBase implements DiffLayoutInterface,
   +        ]))->toString(),
   

   unrelated change.

2. +++ b/src/DiffLayoutBase.php
   @@ -126,8 +185,8 @@ abstract class DiffLayoutBase extends PluginBase implements DiffLayoutInterface,
   -
   -  /**
   +  ¶
   +      /**
   

   Indentation change unrelated.

As suggested, add #access to don't display those elements when the user doesn't have access/enough permissions.

Some more feedback details:

1. +++ b/src/DiffLayoutBase.php
   @@ -93,7 +93,66 @@ abstract class DiffLayoutBase extends PluginBase implements DiffLayoutInterface,
   +      //Creating author URL.
   ...
   +       //Creating revision date URL.
   
   +++ b/src/Plugin/diff/Layout/VisualInlineDiffLayout.php
   @@ -166,6 +166,19 @@ class VisualInlineDiffLayout extends DiffLayoutBase {
   +    //Show the revisions that are compared.
   

   Add a white-space between "//" and your comment.
   Here is the general Drupal API documentation standards.
   And generally it starts as an "imperative verb".

2. +++ b/src/DiffLayoutBase.php
   @@ -93,7 +93,66 @@ abstract class DiffLayoutBase extends PluginBase implements DiffLayoutInterface,
   +      $revision_link['author'] = [
   +        '#type' => 'link',
   +        '#title' => $revision->getRevisionUser()->getDisplayName(),
   +        '#url' => Url::fromUri(\Drupal::request()->getUriForPath('/user/' . $user_id)),
   +        '#attributes' => [
   +          'class' => ['comparation-flex-item-author'],
   +        ],
   +      ];
   

   As mentioned in the previous comments, you will need to wrap this block into an if checking for the "View user information" permission like
   $user->hasPermission('access user profiles').

   And you can do this like in RevisionOverviewForm::buildForm():

   $username = [
               '#theme' => 'username',
               '#account' => $revision->getRevisionAuthor(),
             ];
   

3. +++ b/src/DiffLayoutBase.php
   @@ -126,8 +185,8 @@ abstract class DiffLayoutBase extends PluginBase implements DiffLayoutInterface,
   +  ¶
   +      /**
   

   Reformat/revert this, as also John said.

4. +++ b/src/Plugin/diff/Layout/VisualInlineDiffLayout.php
   @@ -166,6 +166,19 @@ class VisualInlineDiffLayout extends DiffLayoutBase {
   +    $build['diff_revisions']['left_revision'] = $this->buildRevisionLinkVisualInline($left_revision);
   +    $build['diff_revisions']['left_revision']['#prefix'] = '<div class="comparation-flex-container">';
   +    $build['diff_revisions']['left_revision']['#suffix'] = '</div>';
   +    $build['diff_revisions']['right_revision'] = $this->buildRevisionLinkVisualInline($right_revision);
   +    $build['diff_revisions']['right_revision']['#prefix'] = '<div class="comparation-flex-container">';
   +    $build['diff_revisions']['right_revision']['#suffix'] = '</div>';
   

   Let's do these as two blocks like

   $build['diff_revisions']['left_revision'] = [
     '#prefix' => ...
     '#suffix' => ...
   ];
   

   to make these line length less than 80 (?)

+++ b/src/Plugin/diff/Layout/VisualInlineDiffLayout.php
@@ -166,6 +166,19 @@ class VisualInlineDiffLayout extends DiffLayoutBase {
+      'weight' => 0,

please, don't forget the # before weight.

Added changes from previous comments. I also provided screenshots for different resolutions.

I actually realized that the css shouldn't be in the general css. Will change it.

Now placed the code for be more plugin specific, we don't want to mix functions between layouts ;).

I also noticed that. Follow-up created.