Problem/Motivation

Discussed with @miro_dietiker, we think this information - that the cropping is not applied, is too much/not needed and just adds unuseful information to the UI.

Proposed resolution

Remove the text "No croppin applied".

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#5 display_only_cropping-2631626-5-interdiff.txt693 bytessasanikolic
#5 display_only_cropping-2631626-5.patch720 bytessasanikolic
#2 display_only_cropping-2631626-2.patch545 bytessasanikolic
Screen Shot 2015-12-09 at 11.12.13.png21.98 KBsasanikolic

Comments

sasanikolic created an issue. See original summary.

sasanikolic’s picture

sasanikolic commented
Status: Active » Needs review
StatusFileSize
new display_only_cropping-2631626-2.patch545 bytes
slashrsm’s picture

slashrsm commented 
+++ b/js/imageWidgetCrop.js
@@ -122,7 +122,7 @@
-    var summaryText = Drupal.t('No cropping applied');
+    var summaryText = null;

I am not super JS expert and don't entirely know how picky JS is about types, but should we rather use an empty string instead of null?

luksak’s picture

luksak
he/him
commented

Well, we do not need to set this at all if we don't want this behavior. But I thinks this information can be useful. Why did you change your mind?

sasanikolic’s picture

sasanikolic commented
StatusFileSize
new display_only_cropping-2631626-5.patch720 bytes
new display_only_cropping-2631626-5-interdiff.txt693 bytes

Second version, without the variable. I think it looks cleaner now. @miro?

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented

Reduction of visual noise is a significant concept.
Writing that "No cropping applied" is adding noise without relevant information.
The relevant information is "Cropping applied", because it originates user activity / modifications / special selection.

Thus by default showing less = nothing is perfect and makes the user much more aware if (and which ones) specific vertical tabs have a cropping applied.

luksak’s picture

luksak
he/him
commented

Ah, sorry, I misunderstood the issue. I thought you wanted to remove both strings. I totally agree with this.

luksak’s picture

luksak
he/him
commented
Status: Needs review » Reviewed & tested by the community

Works great.

woprrr’s picture

woprrr commented
Status: Reviewed & tested by the community » Fixed
Issue tags: +D8Media

Work well :) Thanks!

  • woprrr committed dea28d9 on 8.x-1.x authored by sasanikolic 
    Issue #2631626 by sasanikolic, Lukas von Blarer, slashrsm, miro_dietiker...

Status: Fixed » Closed (fixed)

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