[CLOSED] Preview Images

core-ai-bot commented 3 years ago

Issue by couzteau Friday Oct 11, 2013 at 22:34 GMT Originally opened as https://github.com/adobe/brackets/pull/5495

This implementation provides a new class namely ImageViewer that enables EditorManager to display images in the editor-holder container.

Implementation highlights:

DocumentCommmandHandler owns the decision of opening a file in an Editor or in a custom viewer. -
EditorManager maintains the view state in a new local variable _currentlyViewedFile and associated and exported getter and setter methods for the same to store path to the file in view, i.e. a text document in the editor or the path in the image viewer. ProjectManager no longer uses getCurrentDocument to maintain the project tree, but uses getCurrentFile instead.
To determine whether a file is an image or a text document LanguageManager's method getLanguageForPath should be used. For image files it detects a new mode with id 'image'.
Changing the file in view always fires a _onCurrentDocumentChange-event. This also happens when the user switches views from one image to another. EditorManager's event handler for _onCurrentDocumentChange holds the logic to show and hide ImageViewers similar to the way editors are hidden and shown.
ImageViewer is the only custom viewer at this point, but the design is made to make the addition of more custom viewers easy, i.e. SVG viewer, video player, audio player, ...? To keep EditorManager code independent of ImageViewer implementation details, only the method showCustomViewer and the resize event handler have references to image viewer. These are wrapped with a language / mode test, so that these areas can easily be extended to accomodate othe viewers.
References:
Trello: https://trello.com/c/l9AcILkC/24-8-preview-images
Spec: https://github.com/adobe/brackets/wiki/Preview-Images-Spec

Note: The code in this review does not yet include new unit tests and some tweaks to drag and drop and file open. That said drag & drop and file open do work. Currently drag&drop and file open will erroneously open images into the working set. In that scenario file close does not work properly. These issues will be fixed by Raymond in a 2nd pull request

couzteau included the following code: https://github.com/adobe/brackets/pull/5495/commits

core-ai-bot commented 3 years ago

Comment by couzteau Friday Oct 11, 2013 at 22:54 GMT

Note on the Travis failure: I'm not sure why it complains when running JSHint:

Running "jshint:all" (jshint) task
Linting src/document/DocumentManager.js...ERROR
[L580:C13] W117: '_clearCurrentDocument' is not defined.
            _clearCurrentDocument();

I though we only use JSLint

core-ai-bot commented 3 years ago

Comment by njx Friday Oct 11, 2013 at 23:01 GMT

We run jshint in travis, but the errors should generally be the same. I'm not sure why it's complaining here though since that line doesn't call _clearCurrentDocument() anymore. Maybe just try restarting the Travis build?

core-ai-bot commented 3 years ago

Comment by couzteau Friday Oct 11, 2013 at 23:07 GMT

@njx the error reproduces, there are tests that define a method: _clearCurrentDocument

core-ai-bot commented 3 years ago

Comment by couzteau Friday Oct 11, 2013 at 23:30 GMT

Still weird. when I run jshint document/* locally there are no errors. I'm pretty sure though that this error is related to renaming the function _clearCurrentDocument to clearCurrentDocument to export it

core-ai-bot commented 3 years ago

Comment by MarcelGerber Saturday Oct 12, 2013 at 09:04 GMT

I just got an seious bug on switching files:

(faaa25b2a35c8221c30d9505732d4b3769bf53ef)

core-ai-bot commented 3 years ago

Comment by couzteau Monday Oct 14, 2013 at 17:14 GMT

@SAPlayer thanks. I'll take a look

core-ai-bot commented 3 years ago

Comment by peterflynn Monday Oct 14, 2013 at 17:25 GMT

@SAPlayer How did you switch files? Does it happen literally every time you view an image and then switch to a text file? Or only when you do the switch in certain ways?

core-ai-bot commented 3 years ago

Comment by MarcelGerber Monday Oct 14, 2013 at 18:37 GMT

I don't know how this has happened, so no, I have no steps to repro. It happened only once, after I switched the file with the mouse. Maybe I can try to find repro steps. I think it was a (almost) clear installation.

core-ai-bot commented 3 years ago

Comment by peterflynn Monday Oct 14, 2013 at 18:40 GMT

@SAPlayer If it happens again, might want to check the console for errors too (Debug > Show Developer Tools). If there are any exceptions, expand the triangle so you can copy the whole stack trace...

core-ai-bot commented 3 years ago

Comment by couzteau Monday Oct 14, 2013 at 19:12 GMT

Fixed SAPlayers issue. It was quite easy to repro, because sit would happen the first time you'd switch from teh image in the getting started project to the index.html file in the working set.

core-ai-bot commented 3 years ago

Comment by MarcelGerber Monday Oct 14, 2013 at 19:14 GMT

Yes, seems to work.

I was just able to repro this issue with the build before.

core-ai-bot commented 3 years ago

Comment by couzteau Monday Oct 14, 2013 at 19:28 GMT

@SAPlayer thanks for verifying

core-ai-bot commented 3 years ago

Comment by couzteau Tuesday Oct 15, 2013 at 22:21 GMT

Closing this PR to open a new one

brackets-archive / bracketsIssues

[CLOSED] Preview Images #5047