-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Exempt javascript files from files check #1143
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
Conversation
There are various |
Perhaps both "edit" and "raw" links would be appropriate. |
Yeah, there was a similar issue for json files recently. I think adding an explicit way for the user to open any file in the editor makes sense (perhaps with a blacklist of mimetypes where we know that's not useful, like |
The edit page should behave sensibly if you give it an image/png ("I couldn't decode ..."), if we want to be lazy about the edit link. |
Or create a new window displaying the image. We have the information needed to display common image types: https://github.com/jupyter/jupyter-js-plugins/blob/master/src/imagehandler/plugin.ts#L42 |
Makes sense that that would happen.
I think the implementation I'm gonna go for is placing a button on the file if and only if it is editable. That way we avoid the user having to make a bad choice and the UI is cleaner. Do we want to close this and start over or merge and I can submit another PR? |
I think start over - I don't think there's much value in fixing it for specific special cases.
How are you planning to decide what's editable? The bigger problem is that there's no reliable way of knowing what's editable without reading the files. |
I think also having an explicit way to "raw load" any file (e.g. what happens currently when you click the link to a JavaScript file) would also be helpful, similar to how GitHub provides both "raw" and "edit" (pencil icon) buttons. Obviously, not all of those ^ buttons make sense for Jupyter notebooks, but something like that. It would be important that raw access provide/preserve the appropriate mimetype for any given file. If one chooses to click the edit button for a file that the editor can't load, I imagine there's some type of exception that the editor can/does throw when it tries to load it, which could be caught and an appropriate error-message displayed, superimposed on the editor-area of the edit page (instead of the file-folder navigation page). I think this would also lend itself to the editor context being more pluggable, e.g. it could be possible at some point (via custom configuration/scripts) to invoke an editor such as DarkroomJS rather than the default text editor when the file being loaded has an image mimetype. If logic in the the file-folder navigation context is trying to decide what's edit-able, it seems like it would just get in the way or require adjustments in multiple places. |
I like the view raw menu item idea. We could default to rendering anything below a maximum file size limit in the editor with the view raw rendering button/menu item and anything above the limit would be handled by the browser directly. For now, seems like @captainsafia 's PR is a good band-aid. May we merge as-is while we continue to discuss? |
Exempt javascript files from files check
I'll open up a WIP PR to follow-up on some of the discussions that we had here. The PR will focus on fixing the edit functionality on the file tree. While I think the "Raw" button is a good feature for some, I think it should probably be provided as an extension to the Notebook instead of a baked-in feature. Most users are used to downloading raw files using File > Download interface and an extra button to accomplish the same thing might be confusing for some. Since making notebook extensions is my new obsession, I opened up a repository for it here. I think I should have something for you by Thursday night, @michaelsbradleyjr. |
Closes #1040. External JS files are loaded with `application/javascript` mimetype. cc: @Carreau @michaelsbradleyjr
Closes jupyter#1040. External JS files are loaded with `application/javascript` mimetype. cc: @Carreau @michaelsbradleyjr
Closes #1040.
External JS files are loaded with
application/javascript
mimetype.cc: @Carreau @michaelsbradleyjr