For this assignment I have selected issue #451


contextmenu
This issue consists of including copy, cut, and paste into the context menu for the editor. As well as include a check to see if the browser can support said commands, if not it should post a dialog.

During this adjustment I used https://github.com/adobe/brackets/pull/12674/files for reference as they have already implemented this modification. I started working on the dialog menu which ended up looking like this

the scripts to make this dialog open and quit seemed a bit off, but I had the initial hopes of changing it once I got it working. Once I completed it and added functionality to test if the commands are supported using document.queryCommandSupported(), I did some research on how to include my html file into the EditorCommandHandlers.js file and figured out that this method wouldn’t work. Once I realized I hit a dead end I started on a new path which was creating a modal where html code is appended into a var and calling modal to display it onto the current page. I haven’t gotten to give this a chance yet though. I am planning on trying it tomorrow.

I also decided to check to see if everything was working as I held it back for a while. I noticed that paste was no longer working since I made changes playing around with things. This is also on my to do list. Checking all the work I have done I was unable to solve the issue. I am planning on repulling and attempting to make the changes again to solve this issue.

UPDATE: I implemented the above, which worked as intended. Once this was resolved I noticed that the paste command which should return false when inputted into the document.queryCommandSupported() method when it should return false in chrome. After discussing the issue with David Humphrey we decided that paste shouldn’t use the document.queryCommandSupported() method and should always be treated as if it wasn’t supported by the browser.

Once the dialog was functioning correctly. I was on to fixing the next issue which consisted of the native shortcuts displaying the dialog box. The solution for this consisted of removed the shortcut from the keyboard.json file. They were unnecessary as the shortcuts should be handled naively. Once they were removed the code worked as intended.

After this was implemented and working it was just a matter of adjusting the spacing. I attempted to use npm test to use the linter in order to identify all my incorrect stylings. There was an issue that prevented the testing automation where files were missing(first instance was something called jasmine). I attempted deleting and repulling and  rerunning all the commands  where id then get a different issue. After I repulled from brackets and installed all the extensions, it finally worked. Though the linter didn’t point out any spacing inconsistencies. Where I’d accidentally mix tabs and spaces, or other basic issues. After this I decided to manually skim through all the code and carefully correct the styling.

The pull request is now pending and I am waiting to hear back from my reviewers. I will update the blog again with my conclusion about working on this issue.

UPDATE: the pull request was merged on April 10th, 2017

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s