Merge lp://qastaging/~elementary-pantheon/scratch/confirm-heavy-files into lp://qastaging/~elementary-apps/scratch/scratch

Proposed by Corentin Noël
Status: Needs review
Proposed branch: lp://qastaging/~elementary-pantheon/scratch/confirm-heavy-files
Merge into: lp://qastaging/~elementary-apps/scratch/scratch
Diff against target: 50 lines (+27/-1)
1 file modified
src/Services/Document.vala (+27/-1)
To merge this branch: bzr merge lp://qastaging/~elementary-pantheon/scratch/confirm-heavy-files
Reviewer Review Type Date Requested Status
Jeremy Wootten ux Needs Fixing
Review via email: mp+304834@code.qastaging.launchpad.net

Description of the change

If it's too heavy, Scratch is crashing

To post a comment you must log in.
Revision history for this message
Danielle Foré (danrabbit) wrote :

Maybe a couple inline comments

Revision history for this message
Corentin Noël (tintou) wrote :

Okay, here is a more elegant version :)

1747. By Corentin Noël

 Ask to open very big files.

Revision history for this message
Danielle Foré (danrabbit) wrote :

I'm not sure this is a great metric. For example the Scratch binary is under 1MB as are a number of other apps. It catches the one case, I dunno if it completely solves the issue of dropping in a binary.

I'm also not sure AlertView is the right widget for this. There's no obvious way to cancel.

Revision history for this message
Corentin Noël (tintou) wrote :

This is not fixing the binary problem but loading very large files that just make Scratch crash. Well as any file you open, the way to "cancel" is to close the tab.

Revision history for this message
Jeremy Wootten (jeremywootten) wrote :

I needed to merge the trunk before it would build (although there were no conflicts).

Since AlertView is presumably intended for some kind of warning, it is strange that the option for a cancel button is missing but that should be fixed in Granite if required. It would also make sense for the font sizes to match those in the Welcome widget.

It would be good to combine the check with one for content-type but that could be left to another branch.

The branch works as described but in my opinion the phrase "very heavy files" may be mistranslated and should be something more specific such as "files exceeding 1Mb in size" so the user knows which files they *can* open.

review: Needs Fixing (ux)

Unmerged revisions

1747. By Corentin Noël

 Ask to open very big files.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches