Problem/Motivation

From #2801749: Unclear what is compared with the visual plugin that provides an indication of which revisions are currently compared (for the Visual inline filter) and after a discussion with @miro_dietiker, the "underline-link" for the author a-Element is taking its whole space and it is centered, which is not a usual Drupal output.

Proposed resolution

The author a-Element should be wrapped by a container/div, and it should not be centered.

Remaining tasks

User interface changes

Change how the revision author link is rendered in revision comparison indications for the Visual inline filter.

Now:
how_it_is

How it should look like:
how_it_should_be

API changes

Data model changes

CommentFileSizeAuthor
#14 interdiff-2812913-12-14.txt1.2 KBdrobnjak
#14 reformat_how_the-2812913-14.patch1.94 KBdrobnjak
#12 interdiff-2812913-10-12.txt2.3 KBdrobnjak
#12 reformat_how_the-2812913-12.patch3.04 KBdrobnjak
#12 Screenshot from 2016-10-10 17-40-58.png51.99 KBdrobnjak
#10 Screenshot from 2016-10-07 10-34-56.png68.63 KBdrobnjak
#10 interdiff-2812949-8-10.txt972 bytesdrobnjak
#10 reformat_how_the-2812913-10.patch2.2 KBdrobnjak
#8 interdiff-2812913-3-8.txt926 bytesdrobnjak
#8 reformat_how_the-2812913-8.patch1.67 KBdrobnjak
#8 Screenshot from 2016-10-06 17-20-39.png22.9 KBdrobnjak
#6 Screenshot from 2016-10-06 17-07-33.png36.15 KBdrobnjak
#5 Screenshot from 2016-10-06 10-17-44.png75.14 KBjohnchque
#3 Screenshot from 2016-10-06 16-01-13.png32.29 KBdrobnjak
#3 reformat_how_the-2812913-3.patch1.06 KBdrobnjak
how_it_should_be.png27.66 KBtduong
how_it_is.png28 KBtduong

Comments

tduong created an issue. See original summary.

drobnjak’s picture

drobnjak commented
Assigned: Unassigned » drobnjak
drobnjak’s picture

drobnjak commented
StatusFileSize
new reformat_how_the-2812913-3.patch1.06 KB
new Screenshot from 2016-10-06 16-01-13.png32.29 KB

Author link wrapped in the div element and "text-align: center" removed.

drobnjak’s picture

drobnjak commented
Status: Active » Needs review
johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented
Status: Needs review » Needs work
StatusFileSize
new Screenshot from 2016-10-06 10-17-44.png75.14 KB

Looks good but is it me or the users link are sightly disaligned?

drobnjak’s picture

drobnjak commented
StatusFileSize
new Screenshot from 2016-10-06 17-07-33.png36.15 KB

For me works completely fine. Tested with different resolutions.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented

No, it will not if the values of the revision date has an hour like 11:11, can you check again?

drobnjak’s picture

drobnjak commented
Status: Needs work » Needs review
StatusFileSize
new Screenshot from 2016-10-06 17-20-39.png22.9 KB
new reformat_how_the-2812913-8.patch1.67 KB
new interdiff-2812913-3-8.txt926 bytes

Date is now wrapped in the div. Screenshot provided with the latest change.

tduong’s picture

tduong commented
Status: Needs review » Needs work

Strange hour (first revision's date) but I guess you've just deleted some chars by mistake xd

Ok, can you shorten the space between the revision date and the username links, so it won't look like "far away" please ? :P

drobnjak’s picture

drobnjak commented
Status: Needs work » Needs review
StatusFileSize
new reformat_how_the-2812913-10.patch2.2 KB
new interdiff-2812949-8-10.txt972 bytes
new Screenshot from 2016-10-07 10-34-56.png68.63 KB

Changes to css for different resolutions and devices. Screenshot provided for one of them.
And yes, character was deleted but on purpose to show how different numbers in dates wont affect the user columns to be unaligned.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented
Status: Needs review » Needs work 
  1. +++ b/css/diff.visual-inline.css
@@ -12,17 +12,16 @@ ins {
 .comparation-flex-item-date {

    Let's add a padding-right: 1em;

  2. +++ b/css/diff.visual-inline.css
@@ -12,17 +12,16 @@ ins {
+  flex-basis: 18%;

    Let's change this to 20% 

  3. +++ b/css/diff.visual-inline.css
@@ -12,17 +12,16 @@ ins {
 .comparation-flex-item-author {

    Also add here the padding_right 1em; 

  4. +++ b/css/diff.visual-inline.css
@@ -36,3 +35,21 @@ ins {
+@media screen and (min-width: 920px) and (max-width: 1100px) {
+  .comparation-flex-item-date {
+    flex-basis: 15%;
+  }
+  .comparation-flex-item-author {
+    flex-basis: 15%;
+    padding: 0 1em 0 1em;
+  }
+}
+@media screen and (min-width: 1100px) {
+  .comparation-flex-item-date {
+    flex-basis: 15%;
+  }
+  .comparation-flex-item-author {
+    flex-basis: 15%;
+    padding: 0 0 0 0;
+  }

    This would make the date too narrow and place them in two lines, let's avoid too many changes for different screens. Adding the message as basis 80% will place it in a new line, that is nice enough IMO.

If possible remove the min-width so we base our width on the flex-basis.

drobnjak’s picture

drobnjak commented
Status: Needs work » Needs review
StatusFileSize
new Screenshot from 2016-10-10 17-40-58.png51.99 KB
new reformat_how_the-2812913-12.patch3.04 KB
new interdiff-2812913-10-12.txt2.3 KB
miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Status: Needs review » Needs work 
  1. +++ b/composer.json
@@ -3,6 +3,12 @@
+  "support": {
+    "issues": "https://www.drupal.org/project/issues/diff"
+  },

@@ -33,8 +39,5 @@
   "support": {
     "issues": "https://www.drupal.org/project/issues/diff",
     "source": "http://cgit.drupalcode.org/diff"
-  },

    You changed it and now support is present twice, making it strictly malformed. 

  2. +++ b/src/Plugin/diff/Layout/VisualInlineDiffLayout.php
@@ -136,7 +136,7 @@ class VisualInlineDiffLayout extends DiffLayoutBase {
-      '#weight' => 0,
+      'weight' => 0,

    You are reverting this, which was previously fixed in some other issue.
    Please properly reroll / rebase your work.

drobnjak’s picture

drobnjak commented
Status: Needs work » Needs review
StatusFileSize
new reformat_how_the-2812913-14.patch1.94 KB
new interdiff-2812913-12-14.txt1.2 KB

Updated changes from #13

  • miro_dietiker committed 155646d on 8.x-1.x authored by drobnjak 
    Issue #2812913 by drobnjak, tduong, yongt9412: Reformat how the revision...
miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Status: Needs review » Fixed

Yeah it can be that simple. Committed. :-)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.