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

Prevent overlapping description in medium sized mobile views #471

Merged
merged 7 commits into from Feb 9, 2019

Conversation

@pbauer
Copy link
Contributor

pbauer commented Feb 5, 2019

This fixes several issues:

  • On a medium sized mobile view the description was overlapping with the columns 'Size' and 'Last Modified' makeing that hard to read
  • When using Products.ExternalEditor a pencil-link to edit a object is added after the title but the description was rendered over that link maing it unuseable.
  • With this change the descriptions is rendered in the next line when it is so long that it would overlap with anything.

Before:
before

After:
after

@pbauer pbauer requested review from drfho and icemac Feb 5, 2019

@ale-rt
Copy link
Member

ale-rt left a comment

LGTM

Better reviewed removing the whitespaces from the diff https://github.com/zopefoundation/Zope/pull/471/files?w=1

@drfho

This comment has been minimized.

Copy link
Contributor

drfho commented Feb 5, 2019

The table styling has to provide a good compromise between clear layout / quick orientation and completeness of the shown data. This is archived by a monotonous table grid and avoiding any jumping of the row heights. The additional pencil icon should be rendered grid conformant (i.e. as additional table column). Actually there are some situations near the medium-breakpoint where an unpleasant text overlap by long titles can occur. Maybe someone has an idea, how to avoid that by more sophisticated CSS or by JS?
My proposal would be a slight modification of the CSS (shifting the nowrap/ellipsis properties to the cell element:

.zmi table.objectItems tbody .zmi-object-id {
    font-weight: 600;
+    max-width: calc(100vw - 340px);
+    overflow: hidden;
+    white-space: nowrap;
+    text-overflow: ellipsis;
+    color: silver !important;
}
.zmi table.objectItems .zmi-object-title {
    font-weight: 400;
    opacity: .5;
-    /* position: absolute; */
-    /* white-space: nowrap; */
-    /* text-overflow: ellipsis; */
-    /* overflow: hidden; */
-    /* max-width: 60%; */
-    /* padding-left: .25em; */
}
Dr. Frank Hoffmann Dr. Frank Hoffmann
@icemac

This comment has been minimized.

Copy link
Member

icemac commented Feb 7, 2019

@pbauer Could you please check against the commit of @drfho?

@icemac icemac added this to In progress in Zope 4 final release via automation Feb 7, 2019

@icemac icemac added this to the 4.0 final milestone Feb 7, 2019

@drfho

This comment has been minimized.

Copy link
Contributor

drfho commented Feb 7, 2019

@pbauer : ... and for simulating a grid conformant implanting of the ExternalEditor icon here a tentatively code snnippet in JavaScript you can evaluate per browser webdev-tool:

 $('table.objectItems').find('tr').each(function(){
		$(this).find('th').eq(1).after('<th>&nbsp;</th>');
		var random_boolean = Math.random() >= 0.5;
		if (random_boolean) {
			$(this).find('td').eq(1).after('<td><i title="External Editor" class="far fa-edit"></i></td>');
		} else {
			$(this).find('td').eq(1).after('<td>&nbsp;</i></td>');
		}
        
   });

zmi_grid

@icemac

This comment has been minimized.

Copy link
Member

icemac commented Feb 7, 2019

@drfho This looks nice. It requires changes in ExternalEditor with overwrites the templates to add its link. Maybe it should be switched to do it using JavaScript as it would to drop the existing monkey patch mechanism. It could be included using the mechanism described in https://zope.readthedocs.io/en/latest/ZMI.html#use-custom-icons-and-resources.
Someone volunteering to create a PR in Products.ExternalEditor?

Zope 4 final release automation moved this from In progress to Reviewer approved Feb 7, 2019

@icemac
Copy link
Member

icemac left a comment

LGTM.

icemac added some commits Feb 9, 2019

Add change log.
[skip ci]

@icemac icemac dismissed their stale review via eed2218 Feb 9, 2019

Zope 4 final release automation moved this from Reviewer approved to Needs review Feb 9, 2019

@icemac icemac merged commit b92ba17 into master Feb 9, 2019

Zope 4 final release automation moved this from Needs review to Done Feb 9, 2019

@icemac icemac deleted the zmi_description_not_absolute branch Feb 9, 2019

@icemac icemac referenced this pull request Feb 9, 2019

Open

Adapt to new ZMI #13

@icemac

This comment has been minimized.

Copy link
Member

icemac commented Feb 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment