Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Context Menu API, partial implementation #1012

Merged
merged 50 commits into from
Jun 14, 2012
Merged

Conversation

tvoliter
Copy link
Contributor

@tvoliter tvoliter commented Jun 7, 2012

Ready for review

@mikechambers
Copy link
Contributor

Can you provide a simple example of how an extension would create and open a context-menu?

* Represents a context menu that can open at a specific location in the UI. Call getMenu()
* to access the Menu object for adding MenuItems.
*
* Clients should nowtcreate this object directly and should instead use registerContextMenu()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: nowtcreate

@tvoliter
Copy link
Contributor Author

tvoliter commented Jun 7, 2012

Sure, here is a hello world demo you can paste into a main.js file to try it out as a plugin

/_jslint vars: true, plusplus: true, devel: true, nomen: true, regexp: true, indent: 4, maxerr: 50 */
/_global define, $, brackets, window */

/** Simple extension that adds a "File > Hello World" menu item */
define(function (require, exports, module) {
'use strict';

var CommandManager = brackets.getModule("command/CommandManager"),
    Menus          = brackets.getModule("command/Menus");


// Function to run when the menu item is clicked
function handleHelloWorld() {
    window.alert("Hello, world!");
}


// First, register a command - a UI-less object associating an id to a handler
var MY_COMMAND_ID = "helloworld.sayhello";
CommandManager.register("Hello World", MY_COMMAND_ID, handleHelloWorld);

// Register a new context menu
var cmenu = Menus.registerContextMenu("helloworld-context-menu");

// Get the menu object related to the context menu and add a MenuItem that will call your command
var menu = cmenu.getMenu();
menu.addMenuItem("mymenuid", MY_COMMAND_ID);

// Trigger the context meny by calling open(). Here we show it when right-clicking over the
// #projects div
$("#projects").mousedown(function (e) {
        if (e.which === 3) {
            cmenu.open(e.pageX, e.pageY);
        }
    });

});

-----Original Message-----
From: Mike Chambers [mailto:reply@reply.github.com]
Sent: Thursday, June 07, 2012 2:08 PM
To: Ty Voliter
Subject: Re: [brackets] Context Menu API (for review only) (#1012)

Can you provide a simple example of how an extension would create and open a context-menu?


Reply to this email directly or view it on GitHub:
#1012 (comment)

@mikechambers
Copy link
Contributor

Could the x,y coords to open be optional, and if they are not passed in default to the correct position relative to the current position of the mouse?

Otherwise, there may be situations where the position of the menu should be different depending on the platform, that extension developers might not know about, or might not take into account.

Plus, it covers the primary use case and would make the api easier to use.

@redmunds
Copy link
Contributor

redmunds commented Jun 8, 2012

@mikechambers On Windows, you can use the context-menu button to open a context-menu for the currently selected element (not at the mouse), so this is so both the keyboard and the mouse can be supported.

After that, Brackets will attempt to display the menu at the specified coordinates, but will adjust the menu position as necessary to fit it in the Brackets window.

@mikechambers
Copy link
Contributor

@redmunds Sorry. What is the context-menu button?

@tvoliter
Copy link
Contributor Author

tvoliter commented Jun 8, 2012

Great suggestion!

-----Original Message-----
From: Mike Chambers [mailto:reply@reply.github.com]
Sent: Thursday, June 07, 2012 4:54 PM
To: Ty Voliter
Subject: Re: [brackets] Context Menu API (for review only) (#1012)

Could the x,y coords to open be optional, and if they are not passed in default to the correct position relative to the current position of the mouse?

Otherwise, there may be situations where the position of the menu should be different depending on the platform, that extension developers might not know about, or might not take into account.

Plus, it covers the primary use case and would make the api easier to use.


Reply to this email directly or view it on GitHub:
#1012 (comment)

@mikechambers
Copy link
Contributor

@redmunds Thanks. I didn't realize that you were referring to the context menu keyboard button on some windows keyboards. (I didn't even know such a thing existed).

@mikechambers
Copy link
Contributor

What happens if multiple extensions want to work with the context menu and / or add items to it? For example, imagine a context menu opened when text selected and the user right clicks it. How will multiple extensions be able to add items to a single context menu?

Also, will there be default items (copy, cut, paste) that will always be in the context menu?

@tvoliter
Copy link
Contributor Author

tvoliter commented Jun 8, 2012

These are great questions and this is what I am actually investigating next. Early thoughts were that Brackets would have a few built in context menus tied to specific click areas like the editor, working set, file tree, etc. Plugins could add to these and could be more context sensitive by disabling/enabling items based on some criteria when the beforeContextMenuOpen event is triggered. I am also exploring how event bubbling and context menus will interact to see how Brackets will handle a general context menu that shows when clicking over a large div and a very specific context menu that shows when clicking on some small portion of that div.

What are your expectations on how this should work?

-----Original Message-----
From: Mike Chambers [mailto:reply@reply.github.com]
Sent: Thursday, June 07, 2012 5:33 PM
To: Ty Voliter
Subject: Re: [brackets] Context Menu API (for review only) (#1012)

What happens if multiple extensions want to work with the context menu and / or add items to it? For example, imagine a context menu opened when text selected and the user right clicks it. How will multiple extensions be able to add items to a single context menu?

Also, will there be default items (copy, cut, paste) that will always be in the context menu?


Reply to this email directly or view it on GitHub:
#1012 (comment)

@mikechambers
Copy link
Contributor

I would expect that my extension would add to an existing menu, and would not create a new one from scratch.

@tvoliter
Copy link
Contributor Author

tvoliter commented Jun 8, 2012

I did a bit of research and it seems it's not possible to query the current mouse position in browsers without an event. However, to make this API for friendly I changed it so it can take a MouseEvent or a specific location.

-----Original Message-----
From: Mike Chambers [mailto:reply@reply.github.com]
Sent: Thursday, June 07, 2012 4:54 PM
To: Ty Voliter
Subject: Re: [brackets] Context Menu API (for review only) (#1012)

Could the x,y coords to open be optional, and if they are not passed in default to the correct position relative to the current position of the mouse?

Otherwise, there may be situations where the position of the menu should be different depending on the platform, that extension developers might not know about, or might not take into account.

Plus, it covers the primary use case and would make the api easier to use.


Reply to this email directly or view it on GitHub:
#1012 (comment)

tvoliter and others added 10 commits June 8, 2012 15:09
…liter/context-menus

* origin/tvoliter/context-menus:
  round all corners of context-menus
  round top corners of context menus
  share styles
  Now maintains the horizontal scroll position, as well.
  Changed scrollDelta to scrollDeltaY, and rounded the value out.
  Use :visible to skip the entire directory if it is not visible.
  Fix JSLint error
  Update About dialog sprint number
  Update CodeMirror SHA
  - Make 'Find in Files' result sections collapsible - Fix text spacing glitch with 'Find in Files' panel header
  Update SHA to reference latest CM merge
  Updated to work with merged CodeMirror for sprint 10
  Now scrolls the document to the original position after adjusting the font size up or down.

Conflicts:
	src/command/Menus.js
*
* To make menu items be contextual to things like selection listen for the "beforeContextMenuOpen"
* to make changes to Command objects before the context menu is shown. MenuItems are views of
* Commands and control a MenuItem's name, enabled state, and checked stae.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: "stae"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. I also changed "MenuItems are views of Commands and control a MenuItem's ..." to "MenuItems are views of Commands which control a MenuItem's ..."

redmunds and others added 4 commits June 13, 2012 10:58
…liter/context-menus

* origin/tvoliter/context-menus:
  no need to escape id when building a dom node. fix typos.
  fix comment
  fix alignment
  add extension to test context menus
* origin/master: (33 commits)
  code review comments
  Update comments
  Update CodeMirror SHA
  code review fixes
  Restore Shift-Tab to outdent behavior
  Re-implement performance suite filtering due to RequireJS changes in SpecRunner
  Revert .json file listing of test suites.
  Add ExtensionUtils to root require context
  added a comment explaining why we aren't using jquery deferreds for the done loading notification
  Fix #971 (Find in Files treats \n, \r, etc. as regular expressions) -- Escape backslash too (and simplify escaping regexp to use char class instead of |).
  createTestWindowAndRun now waits for brackets to be completely done loading (so extensions are loaded before tests run)
  added a 'ready' event to brackets that fires when brackets is completely done loading
  Rewrote unit tests for JavaScriptInlineEditor to get JSUtils from the require context that was used to load the extension. We only need to do this in unit tests where we rely on JSUtils to get the DocumentManager, etc. from the current window.
  Modified ExtensionLoader to allow for retrieving the require context that loaded an extension
  Refactor loadStyleSheet. Change error handling.
  provide way for unit tests to restore commands after a reset
  Fixed bug where triangle wasn't updating correctly by toggling "scroll" events on double click show
  Fix for double-click not working
  Removed unused comments
  Code review fixes.
  ...
@@ -50,3 +50,5 @@

@z-index-brackets-sidebar-resizer: @z-index-brackets-ui + 2;
@z-index-brackets-resizer-div: @z-index-brackets-sidebar-resizer + 1;

@z-index-brackets-context-menu-base: 1000;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be a relative offset as well (some reference + 1) instead of a constant?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was intended to be the reference (or base) that all other context menu z-indexes are based off of. Currently, this is the only one. :)

Would be nice to unify this with the main menus, but currently these index values are hard-coded in bootstrap (even though they are using LESS).

redmunds and others added 16 commits June 13, 2012 14:05
…liter/context-menus

* origin/tvoliter/context-menus:
  udpate comments
  add UL container for context menus
…liter/context-menus

* origin/tvoliter/context-menus:
  cleanup default context menus
* origin/master:
  Pre-filter function list in _findAllMatchingFunctions()
  Updated CodeMirror SHA
  Code review tweaks for JS inline editor.
  Added functionality to restore the font size to its original setting.
  Added 'Restore Font Size'
  Added semi-colons to the end of symbol entities to make them compatible with integers, and added the restore font menu item.
  Added VIEW_RESTORE_FONT_SIZE

Conflicts:
	src/command/Menus.js
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants