Skip to content
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

Confused about what Edit/View does #2203

Closed
ellisonbg opened this issue Feb 19, 2017 · 27 comments · Fixed by #2229
Closed

Confused about what Edit/View does #2203

ellisonbg opened this issue Feb 19, 2017 · 27 comments · Fixed by #2229

Comments

@ellisonbg
Copy link
Contributor

I am doing UI/UX testing for issue #2142. The Edit/View button are either not working or working in a way that is really confusing. Here is what I observe:

  • For notebook, text files (non-html), clicking on the name of the file in the file browser opens the file in the corresponding editor (notebook, file editor).
  • For those files the "Edit" button appears, but does the same thing. All notebook users have gotten used to just clicking on these files, so presenting different way of doing that same thing is confusing and makes it more difficult to find the other buttons they want to use. The button based UI is getting unwieldy with too many buttons that are context dependent. Not suggesting we move away from buttons now, but let's not show ones that are not needed.
  • For HTML files, clicking on the file opens it in the text, editor, clicking the "Edit" button does the same thing. Again, the "Edit" button seems unnecessary. However, if you click "View" you get a new page with a nasty error:

screen shot 2017-02-19 at 11 20 32 am

Is this a bug?

  • For image files, clicking on the file opens it in View mode. The "View" button also shows and does the same thing. In this case, the "View" button is redundant. Furthermore, the "Edit" button shows, but errors after trying to open the file in the text editor.

My recommendations:

  • Remove the "Edit" and "View" buttons in situations where clicking the file already does the right thing.
  • The only usage case that I see where that isn't the case is the "View" on HTML files. I think that makes sense to keep, but we should debug it.

@gnestor

@takluyver
Copy link
Member

We added these because previously you couldn't do either of these things from the UI:

  • View HTML files
  • Edit files that are textual, but where the system is not reporting a text mimetype. This is system dependent, but we've had quite a few reports of this. Since this is to work around our guess of 'can we edit this file', we necessarily have to show the button for some files where it won't work, and it would be weird not to show it on known editable files.

The error viewing an HTML page is a bug; I'll look into that.

takluyver added a commit to takluyver/notebook that referenced this issue Feb 20, 2017
See jupytergh-2203

The URL calculation was going wrong, so it was using a URL starting with
//files. This uses url_path_join() to get the separators right.
@takluyver
Copy link
Member

#2208 fixes the error viewing HTML files, at least for me. Thanks for spotting that before we released!

@Carreau
Copy link
Member

Carreau commented Feb 22, 2017

I think we can close this one as it is html view is fixed by #2208. And I agree with @takluyver that we should not be too smart. Edit/View everywhere seem fine to me.

@ellisonbg
Copy link
Contributor Author

ellisonbg commented Feb 22, 2017 via email

@takluyver
Copy link
Member

They both do something useful under certain conditions; see my earlier comment.

@ellisonbg
Copy link
Contributor Author

Got it! I missed the issue of editable files that report the an unexpected MIME type. But allowing users to click on files that we know they won't be able to edit is very confusing (images in particular). Given this new information, my preference would be:

  • Only offer "View" for HTML files.
  • Show "Edit" always, except for image MIME types.

@takluyver
Copy link
Member

There is another minor use for 'view', which I forgot - it gives you a way to get the raw text of a file which would otherwise open in the text editor. Possibly niche, but could be useful if you have e.g. a very long file.

I'd broadly agree with hiding edit when only images are selected, but we need to be a bit careful, because image/svg+xml, for instance, is text. The media part of mimetypes turns out not to be that helpful for us.

@Carreau
Copy link
Member

Carreau commented Feb 22, 2017

Only offer "View" for HTML files.

Why ? Html are perfectly valid text files. Can't you edit a template.html file that you use from Python in Jupyter.

Show "Edit" always, except for image MIME types.

Why ? Isn't SVG a valid image format you might want to edit with a text editor?

@Carreau
Copy link
Member

Carreau commented Feb 22, 2017

Well, just 🐴 with @takluyver and I think we agreed, we don't want to be in the businesss of deciding what can and/or cannot be viewed/editted.

@Carreau Carreau modified the milestones: 5.1, 5.0 Feb 22, 2017
@ellisonbg
Copy link
Contributor Author

@Carreau sorry I think there were two possible meanings in my sentence "Only offer "View" for HTML files":

  1. For HTML files only offer the View option (not the Edit one)
  2. Only show the View button for HTML files and no other files.

I was meaning (2), so that yes, you would see the "Edit" button for HTML files. But you wouldn't see the "View" button for other things.

And by image types, I was only referring to jpg/png, not svg, which is definitely text and can be edited just fine. Right now if you "edit" a jpg or png, it opens the text editor, but it fails with an encoding error. If a user says "Edit this jpg file" that is a disappointing and confusing outcome...

@takluyver I agree that getting the raw text of a text file using "View" would be useful, but that doesn't work right now. The View button only shows (as far as I can tell) for HTML files. So actually, the current behavior actually matches my preference (View only shows for HTML files).

So I think the only change I would like is to not show the "Edit" button for the image/png and image/jpeg file types.

@Carreau
Copy link
Member

Carreau commented Feb 23, 2017

For HTML files only offer the View option (not the Edit one)
Only show the View button for HTML files and no other files.
I was meaning (2), so that yes, you would see the "Edit" button for HTML files. But you wouldn't see the "View" button for other things.

Ah ! Sorry for the confusion.

but you wouldn't see the "View" button for other things.

Wait do you mean "Edit" ? Like you don't want the "View" button for images ?

And by image types, I was only referring to jpg/png, not svg, which is definitely text and can be edited just fine. Right now if you "edit" a jpg or png, it opens the text editor, but it fails with an encoding error. If a user says "Edit this jpg file" that is a disappointing and confusing outcome...

I got what you were thinking. The problem is that OS are known to not give the right mimetype. We had many issues with windows reporting css as application/css instead of test/css. Starting to black/whitelist is a rats nest. jpeg ? or jpg? or JPEG ? What if photoshop is installed ? Then it will appear as a photoshop file.

Keep in mind that the button at the top are advanced action, by default clicking on the links will do the right thing (hopefully). And removing either edit (or view) has IMHO more chance of annoying advanced users than to really be annoying for newcomers, and I think that Jupyter have mostly been about allowing users to do arbitrary things, and that would go the opposite way.

@ellisonbg
Copy link
Contributor Author

I totally agree we should give advanced users the tools they need. The issue in this case is that the "advanced" usage-case buttons are put right in the middle of very basic buttons such as Move, Download and Delete:

screen shot 2017-02-25 at 6 16 30 pm

That makes the lives of the many non advanced users difficult, just to appease the advanced ones. The right UI for this type of thing would be to put the more advanced action behind a context menu, but I think that is outside the scope of 5.0.

Trying to think about an in between path...

We already have a reg-exp in these code paths for file extensions:

https://github.com/jupyter/notebook/blob/master/notebook/static/tree/js/notebooklist.js#L604

Editing those lists for the Edit/View button is trivial. Also, looking at file extensions is exactly how we map files onto models/views that can view/edit those files in JupyterLab. I can submit a PR to address this...but what to do exactly.

To me, the most important question is this "what precise things do the Edit/View buttons enable a user to do that they could not do otherwise". Based on what Thomas said that is:

  • View an HTML rather than edit it.
  • Edit text files that don't show up as text files.

Let me see if I can put together a PR that shows the Edit/View buttons for these cases alone.

@ellisonbg
Copy link
Contributor Author

OK, I think the PR I just submitted addresses everything. Thanks!

@andyneff
Copy link

I'm still confused. Is this issue resolved? I just upgraded to 5.1.0, and I can not edit what I guess are considered "unknown" files now (but all purely text).

My opinion is to err on side of more flexible. Let anything be editable (with the understandable exception of notebook files, as already done). Maybe use an alert when you click edit to say "Hey, I think this is a binary file, are you sure?", but removing the edit button seem a little drastic to me.

MIME types, accepted extensions, unaccepted extensions (or even other suggestions like binaryornot) all seem to be making the decision for me, when I'd rather decide if I want to be able to click edit or not. Choosing the default behavior is fantastic, but preventing me from editing feels like a little much. Even if I'm only 1% of 1%.

Workaround

I can work around this by adding a bookmark with javascript:$('.edit-button').css('display', 'inline-block') and clicking it every time I want to edit a file that was determined to not be editable.

@takluyver
Copy link
Member

Can you give some examples of files that you expect to be able to edit but can't?

@adl
Copy link

adl commented Sep 29, 2017

@takluyver The GNU Coding standards require all projects to have a set of text files called README, NEWS, ChangeLog, LICENSE, etc. Those are text files without extension. Python's mimetype.guess_type returns (None, None). Jupyter 5.1.0 does not allows those files to be viewed or edited, at least on my system.

As a Unix user, I'm used to think that everything is a text file unless proven otherwise, so this feels backward.

@ellisonbg
Copy link
Contributor Author

ellisonbg commented Sep 29, 2017 via email

@kiendang
Copy link
Member

kiendang commented Oct 4, 2017

I'm also on 5.1.0 and the Edit button doesn't show up for .r and .R files either.

@ellisonbg
Copy link
Contributor Author

ellisonbg commented Oct 4, 2017 via email

@gnestor
Copy link
Contributor

gnestor commented Oct 5, 2017

@ellisonbg Yes, I will push for a 5.2 release tomorrow or Monday with a fix for this.

@ellisonbg
Copy link
Contributor Author

ellisonbg commented Oct 6, 2017 via email

@rgbkrk
Copy link
Member

rgbkrk commented Oct 6, 2017

Could we get the current #2871 in as part of 5.2?

@gnestor
Copy link
Contributor

gnestor commented Oct 6, 2017

@rgbkrk Yes! Shall I merge?

@gnestor
Copy link
Contributor

gnestor commented Oct 9, 2017

Notebook 5.2.0rc1 is available on PyPI so please give it a try and confirm that this is resolved 👍:

pip install notebook --pre --upgrade

@blink1073
Copy link
Contributor

@gnestor, did you mean 5.2.0rc1?

@gnestor
Copy link
Contributor

gnestor commented Oct 9, 2017

I did 🤦‍♂️

@kiendang
Copy link
Member

Just tested. Works for both .r files and files without extension. Thanks a lot @gnestor 👍

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants