feat(gui): Display image size when drive too small #1807

jhermsmeier · 2017-10-24T20:47:09Z

This adds a display of the determined image size to the
drive label when the drive has been determined to be too small.

Change-Type: patch
Changelog-Entry: Display image size in drive selector if drive is too small
Connects To: #1136, #2004

jviotti · 2017-10-24T21:01:50Z

lib/shared/drive-constraints.js

@@ -376,9 +377,10 @@ exports.getDriveImageCompatibilityStatuses = (drive, image) => {
      message: exports.COMPATIBILITY_STATUS_MESSAGES.LOCKED
    })
  } else if (!_.isNil(drive) && !_.isNil(drive.size) && !exports.isDriveLargeEnough(drive, image)) {
+    const imageSize = _.get(image, [ 'size', 'final', 'estimation' ]) ? image.size.final.value : image.size.original


This logic looks weird. Basically, if the image final size is an estimation, use that, otherwise use the original one, which means we will use the original size even for compressed sizes that we know their final size for sure (i.e. that are not estimations).

Maybe the intended logic is: If the final one is not an estimation, use that, otherwise use the original size.

Oh boy, yeah, I messed that up – fixed now :)

Maybe the intended logic is: If the final one is not an estimation, use that, otherwise use the original size.

Isn't that exactly what the 'final' size is anyway?

lurch · 2017-10-25T01:09:12Z

I'm not sure if this is actually needed? In @jhermsmeier 's screenshot above you can clearly see that the (uncompressed as necessary) image size is already shown in the main Etcher GUI anyway...

jhermsmeier · 2017-10-25T11:29:36Z

Yes, you're right – I checked the conversation again, and have now changed it to display the relative bytes missing for the image to fit:

jhermsmeier · 2017-10-25T11:32:21Z

I don't think this is completely unnecessary though, as the issue discusses, it's aimed at helping people figure out why a 8GB image won't fit on a 8GiB device, for example.

lurch · 2017-10-25T11:37:04Z

Instead of "Too Small For Image - 46.15 MB", what about something like "Too Small For Image by 46.15 MB" - might be slightly clearer?

Shou · 2017-10-25T11:38:31Z

I was just about to say the same @lurch! Also maybe we could make the constant into a template that takes a size field?

jhermsmeier · 2017-10-25T11:48:28Z

Yeah, added the "by" :)

Also maybe we could make the constant into a template that takes a size field?

I thought about it when I started this, but the messages used aren't in shared/messages, but the shared/drive-constraints itself – and moving them all over for no gain seems excessive to me. In this case, string concat is more than enough imho, and not worth overcomplicating (also, I'd actually rather get rid of those lodash templates, but that's a different discussion)

konmouz

I agree "Too Small For Image by 46.15 MB" works much better, otherwise, the user has to assume what the MB refers to.

Shou · 2017-10-25T12:56:47Z

(also, I'd actually rather get rid of those lodash templates, but that's a different discussion)

Made #1810 to discuss this

jviotti · 2017-10-25T15:36:44Z

I must confess I'm confused when looking at the screenshot. For example, we have a drive that is 130 MB and the label says "Too small for image by 1.72 GB." I can't tell for sure if it means I should have a drive that is at least 1.72 GB, or that my drive would need to be 1.72 GB larger (in which case I have to do some arithmetic to find that I need a 1.85 GB drive).

Can we rephrase the whole label to something like "Too small. Should be at least XXX MBs" or something like that?

jviotti

Looks good code-wise, but I think the UX still needs to be discussed/polished.

lurch · 2017-10-25T16:04:10Z

Can we rephrase the whole label to something like "Too small. Should be at least XXX MBs" or something like that?

But as I mentioned earlier, that would simply duplicate the (uncompressed) image size, which is already visible in the main Etcher GUI anyway?

Perhaps @jhermsmeier didn't choose the best examples (who has a 130MB drive these days?! 😉 ), but imagine that you have a compressed image that's 8001MB when uncompressed, but your SD card (labelled as "8 GB") actually has a capacity of 8000MB. With @jhermsmeier 's last proposal Etcher would display "Too Small For Image by 1 MB" whereas @jviotti 's proposal would display "Too small. Should be at least 8 GBs"
*shrug*

(P.S. Yes, I can never remember whether Etcher uses 1024 or 1000 when converting from bytes to kilobytes, megabytes, etc. so please ignore any mistake I've made in that regard!)

jhermsmeier · 2017-10-25T17:38:43Z

who has a 130MB drive these days?! 😉

I do :P It's an old marketing gift thing I picked up somewhere, came in handy for testing every now and then :)

P.S. Yes, I can never remember whether Etcher uses 1024 or 1000 when converting from bytes to kilobytes, megabytes, etc. so please ignore any mistake I've made in that regard!

Pretty-bytes (which we're using atm) uses base-10 (aka "hipster-units"), so generally 1KB = 1000B in Etcher (I think there might be an internal place or two where be use 1024B for reasons).

jhermsmeier · 2017-10-25T17:43:08Z

I concur with @lurch – the "Should be at least XXX MB" would just dupe the image size; How about "Insufficient space, additional XX MB required" or something along those lines?

lurch · 2018-01-29T16:45:01Z

This would also fix #2004

jhermsmeier · 2018-01-30T16:43:21Z

Changed the message to 'Insufficient space, additional XX required' & rebased against master

Shou

LGTM after the linter is satisfied

Shou · 2018-02-07T14:21:52Z

lib/shared/drive-constraints.js

    statusList.push({
      type: exports.COMPATIBILITY_STATUS_TYPES.ERROR,
-      message: exports.COMPATIBILITY_STATUS_MESSAGES.TOO_SMALL
+      message: exports.COMPATIBILITY_STATUS_MESSAGES.TOO_SMALL.replace('XX', prettyBytes(relativeBytes))


This is failing with Prefer '_.replace' over the native function lodash/prefer-lodash-method

Shou · 2018-02-07T14:22:38Z

lib/shared/drive-constraints.js

@@ -303,7 +304,7 @@ exports.COMPATIBILITY_STATUS_MESSAGES = {
   * @description
   * The drive is too small for the image.
   */
-  TOO_SMALL: 'Too Small For Image',
+  TOO_SMALL: 'Insufficient space, additional XX required',


Maybe we should make these into template arrow functions? We can keep it as-is now and open a new PR for that, though.

I've now moved the compatibility messages from drive-constraints to shared/messages and made them functions

Shou

LGTM

UX has been updated

jhermsmeier · 2018-02-14T15:43:59Z

@jviotti can you have another look at this?

lurch

Cool :)
Could you update the screenshot in the first comment?

This adds a display of the determined image size to the drive label when the drive has been determined to be too small. Change-Type: patch Changelog-Entry: Display image size for comparison if drive is too small

jhermsmeier requested review from lurch, Shou and jviotti October 24, 2017 20:47

jviotti suggested changes Oct 24, 2017

View reviewed changes

jhermsmeier force-pushed the patch-1136 branch from 4d2d12e to 0a58ae3 Compare October 24, 2017 21:41

jhermsmeier force-pushed the patch-1136 branch from 0a58ae3 to c9e51ec Compare October 25, 2017 11:27

jhermsmeier force-pushed the patch-1136 branch from c9e51ec to 425f180 Compare October 25, 2017 11:31

jhermsmeier requested a review from konmouz October 25, 2017 11:33

jhermsmeier force-pushed the patch-1136 branch from 425f180 to 1166d7c Compare October 25, 2017 11:43

konmouz reviewed Oct 25, 2017

View reviewed changes

jhermsmeier requested a review from jviotti October 25, 2017 11:59

jhermsmeier force-pushed the patch-1136 branch 2 times, most recently from 15f9626 to 64a8c14 Compare October 25, 2017 14:10

jviotti previously requested changes Oct 25, 2017

View reviewed changes

jhermsmeier force-pushed the patch-1136 branch from 64a8c14 to d94039a Compare January 15, 2018 14:39

jhermsmeier force-pushed the patch-1136 branch from d94039a to fac6460 Compare January 30, 2018 16:42

jhermsmeier requested a review from jviotti January 30, 2018 16:43

jhermsmeier requested a review from konmouz January 30, 2018 16:43

Shou approved these changes Feb 7, 2018

View reviewed changes

jhermsmeier force-pushed the patch-1136 branch from fac6460 to bd1adc7 Compare February 13, 2018 16:50

ghost assigned jhermsmeier Feb 13, 2018

ghost added the in progress label Feb 13, 2018

Shou approved these changes Feb 13, 2018

View reviewed changes

konmouz approved these changes Feb 14, 2018

View reviewed changes

jhermsmeier force-pushed the patch-1136 branch from bd1adc7 to 73c7b62 Compare February 16, 2018 15:16

lurch approved these changes Feb 22, 2018

View reviewed changes

jhermsmeier force-pushed the patch-1136 branch from 73c7b62 to c498b5a Compare March 5, 2018 21:33

feat(gui): Display image size when drive too small

4108979

This adds a display of the determined image size to the drive label when the drive has been determined to be too small. Change-Type: patch Changelog-Entry: Display image size for comparison if drive is too small

jhermsmeier force-pushed the patch-1136 branch from c498b5a to 4108979 Compare March 21, 2018 19:36

jhermsmeier merged commit ad7c876 into master Mar 21, 2018

ghost removed the in progress label Mar 21, 2018

jhermsmeier deleted the patch-1136 branch March 21, 2018 22:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(gui): Display image size when drive too small #1807

feat(gui): Display image size when drive too small #1807

jhermsmeier commented Oct 24, 2017 •

edited

jviotti Oct 24, 2017

jhermsmeier Oct 24, 2017

lurch Oct 25, 2017 •

edited

lurch commented Oct 25, 2017

jhermsmeier commented Oct 25, 2017

jhermsmeier commented Oct 25, 2017 •

edited

lurch commented Oct 25, 2017

Shou commented Oct 25, 2017

jhermsmeier commented Oct 25, 2017

konmouz left a comment

Shou commented Oct 25, 2017

jviotti commented Oct 25, 2017

jviotti left a comment

lurch commented Oct 25, 2017 •

edited

jhermsmeier commented Oct 25, 2017

jhermsmeier commented Oct 25, 2017

lurch commented Jan 29, 2018

jhermsmeier commented Jan 30, 2018

Shou left a comment

Shou Feb 7, 2018

jhermsmeier Feb 13, 2018

Shou Feb 7, 2018

jhermsmeier Feb 13, 2018

Shou left a comment

jhermsmeier commented Feb 14, 2018

lurch left a comment

feat(gui): Display image size when drive too small #1807

feat(gui): Display image size when drive too small #1807

Conversation

jhermsmeier commented Oct 24, 2017 • edited

jviotti Oct 24, 2017

Choose a reason for hiding this comment

jhermsmeier Oct 24, 2017

Choose a reason for hiding this comment

lurch Oct 25, 2017 • edited

Choose a reason for hiding this comment

lurch commented Oct 25, 2017

jhermsmeier commented Oct 25, 2017

jhermsmeier commented Oct 25, 2017 • edited

lurch commented Oct 25, 2017

Shou commented Oct 25, 2017

jhermsmeier commented Oct 25, 2017

konmouz left a comment

Choose a reason for hiding this comment

Shou commented Oct 25, 2017

jviotti commented Oct 25, 2017

jviotti left a comment

Choose a reason for hiding this comment

lurch commented Oct 25, 2017 • edited

jhermsmeier commented Oct 25, 2017

jhermsmeier commented Oct 25, 2017

lurch commented Jan 29, 2018

jhermsmeier commented Jan 30, 2018

Shou left a comment

Choose a reason for hiding this comment

Shou Feb 7, 2018

Choose a reason for hiding this comment

jhermsmeier Feb 13, 2018

Choose a reason for hiding this comment

Shou Feb 7, 2018

Choose a reason for hiding this comment

jhermsmeier Feb 13, 2018

Choose a reason for hiding this comment

Shou left a comment

Choose a reason for hiding this comment

jhermsmeier commented Feb 14, 2018

lurch left a comment

Choose a reason for hiding this comment

jhermsmeier commented Oct 24, 2017 •

edited

lurch Oct 25, 2017 •

edited

jhermsmeier commented Oct 25, 2017 •

edited

lurch commented Oct 25, 2017 •

edited