[CLOSED] Disabled context menu items for unsaved files

[core-ai-bot]

Issue by mansimarkaur
Thursday Sep 29, 2016 at 16:44 GMT
Originally opened as adobe/brackets#12806

---

Fixes #11763

---

mansimarkaur included the following code: https://github.com/adobe/brackets/pull/12806/commits

[core-ai-bot]

Comment by ficristo
Thursday Sep 29, 2016 at 17:32 GMT

---

Have you checked what other editors do in these cases?
In general you need some jsdocs.
@petetnt@zaggino could someone of you review?

[core-ai-bot]

Comment by petetnt
Thursday Sep 29, 2016 at 17:56 GMT

---

Thanks for this great PR@mansimarkaur! Tested against latest master and it works great.

Could you add few small pieces of JSDoc for the new methods and maybe simple unit test for the functionality? (it should disable context menu items when not present and it should not disable context menu items when present)

👍

[core-ai-bot]

Comment by mansimarkaur
Sunday Oct 02, 2016 at 16:23 GMT

---

@petetnt sure, I'll add the required changes.

[core-ai-bot]

Comment by mansimarkaur
Thursday Oct 27, 2016 at 17:54 GMT

---

@petetnt Could you please help me with the tests? I was thinking of adding tests in the test/Menu-test.js file. Please let me know if it's right?

[core-ai-bot]

Comment by petetnt
Thursday Oct 27, 2016 at 20:30 GMT

---

@mansimarkaur test/Menu-test.js sounds like a good place.

Any specific things you need help with the tests? The structure should could be something like:

it("it should disable context menu items when file doesn't exist ", function () {
  // runs create a new file
  // opens context menu
  // checks that all the items are disabled
})

it("it should enable context menu items when file does exist ", function () {
  // runs create a new file and saves it
  // opens context menu
  // checks that all the items are enabled
})

For example check DocumentCommandHandlers tests to see how the working set can be manipulated.

[core-ai-bot]

Comment by Denisov21
Saturday Dec 24, 2016 at 15:39 GMT

---

+1 big idea!

[core-ai-bot]

Comment by ficristo
Monday Feb 13, 2017 at 19:25 GMT

---

This needs more work. Closing for now.

[core-ai-bot]

Comment by petetnt
Wednesday Mar 01, 2017 at 19:05 GMT

---

LGTM sans tests, merging this and will open another issue for the tests as per adobe/brackets#11763 (comment)

Thanks for this great contribution@mansimarkaur and sorry for taking so long!

PS: did this test totally flake? https://travis-ci.org/adobe/brackets/builds/206719157 CLA checked manually just in case.