Closed
Bug 1008709
Opened 11 years ago
Closed 10 years ago
Build a preference screen for WebIDE
Categories
(DevTools :: Source Editor, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 33
People
(Reporter: paul, Assigned: paul)
References
Details
Attachments
(1 file, 5 obsolete files)
(deleted),
patch
|
paul
:
review+
|
Details | Diff | Splinter Review |
I want to be able to configure codemirror. Toggle numbers, tabs or spaces, autocomplete brackets, ...
This will be useful for the devtools in general, but it is necessary for the new App Manager where Itchpad (so codemirror) will be a central piece.
Assignee | ||
Comment 1•11 years ago
|
||
Note: this needs to be available outside of a toolbox (the app manager doesn't always have a toolbox open).
Assignee | ||
Comment 2•11 years ago
|
||
Basically, a simple UI for most of these options: http://codemirror.net/doc/manual.html#config
Comment 3•11 years ago
|
||
Pretty sure this is a dupe.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → DUPLICATE
Assignee | ||
Comment 4•10 years ago
|
||
We want something at the WebIDE level. This will most certainly overlaps part of what we want to do in bug 964356, but the overlap will be small.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Summary: Build a preference screen for sourceeditor/codemirror → Build a preference screen for WebIDE
Assignee | ||
Comment 5•10 years ago
|
||
Is there a clean way to reload a projectEditor instance?
Assignee: nobody → paul
Status: REOPENED → ASSIGNED
Attachment #8445120 -
Flags: feedback?(bgrinstead)
Comment 6•10 years ago
|
||
Comment on attachment 8445120 [details] [diff] [review]
v0.1
Review of attachment 8445120 [details] [diff] [review]:
-----------------------------------------------------------------
Not really right now short of destroying then rebuilding all the editors (which could cause loss of unsaved work). I've got a patch that mostly adds this functionality, but there is going to have to be some workarounds for the autocomplete pref, which is handled differently from the others. I'll file a blocker for it
Attachment #8445120 -
Flags: feedback?(bgrinstead)
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #6)
> Not really right now short of destroying then rebuilding all the editors
> (which could cause loss of unsaved work). I've got a patch that mostly adds
> this functionality, but there is going to have to be some workarounds for
> the autocomplete pref, which is handled differently from the others. I'll
> file a blocker for it
Can you check my patch? I added a autocomplete pref.
Comment 8•10 years ago
|
||
(In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from comment #7)
> (In reply to Brian Grinstead [:bgrins] from comment #6)
> > Not really right now short of destroying then rebuilding all the editors
> > (which could cause loss of unsaved work). I've got a patch that mostly adds
> > this functionality, but there is going to have to be some workarounds for
> > the autocomplete pref, which is handled differently from the others. I'll
> > file a blocker for it
>
> Can you check my patch? I added a autocomplete pref.
Yeah, I saw the pref but this is a limitation in the Editor component right now. You can only ever enable the autocompletion, not disable. And if you have already enabled it once it will reinitialize it. I have a patch in Bug 1029511 that will provide this ability, along with a projecteditor.reloadEditorPrefs function that you can call.
Comment 9•10 years ago
|
||
You can apply https://bug1029511.bugzilla.mozilla.org/attachment.cgi?id=8445220 in front of your patch if you want to get it working for now
Comment 10•10 years ago
|
||
**To be applied in series with the patch from Bug 1029511**. This will provide a function called projecteditor.reloadEditorPrefs() that should do what you are wanting.
Assignee | ||
Comment 11•10 years ago
|
||
Brian, it appears to work well, but, autoclosebracket pref doesn't appear to be taken into account, and with I also get this error (not sure if related):
TypeError: popup is null: destroy@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/sourceeditor/autocomplete.js:153:5
Assignee | ||
Comment 12•10 years ago
|
||
Should I keep the autocomplete pref?
Comment 13•10 years ago
|
||
The autoclose brackets is working with the patch in 964356 - I remember there was some discussion about reusing the options js or panel after that bug is resolved. In fact, if we did it that way then the editors would reload prefs immediately as the inputs change instead of needing to be triggered by a function call. What is the reasoning for rolling the prefs functionality separately here instead of reusing some part of what we have in the toolbox?
Comment 14•10 years ago
|
||
>I also get this error (not sure if related):
>
> TypeError: popup is null:
> destroy@resource://gre/modules/commonjs/toolkit/loader.js ->
> resource:///modules/devtools/sourceeditor/autocomplete.js:153:5
What steps do you do to see this error? I'm not able to reproduce
Assignee | ||
Comment 15•10 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #13)
> I remember
> there was some discussion about reusing the options js or panel after that
> bug is resolved. In fact, if we did it that way then the editors would
> reload prefs immediately as the inputs change instead of needing to be
> triggered by a function call. What is the reasoning for rolling the prefs
> functionality separately here instead of reusing some part of what we have
> in the toolbox?
Only the UI would get duplicated. It's not a lot of code.
Assignee | ||
Comment 16•10 years ago
|
||
Attachment #8445120 -
Attachment is obsolete: true
Assignee | ||
Comment 17•10 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #13)
> The autoclose brackets is working with the patch in 964356
I can't make it work.
STR:
- apply v0.2
- open a project
- edit a CSS file
- see that autoclose works
- open preferences
- uncheck autoclose
- click "close"
- in the same CSS file, autoclose is still active
Comment 18•10 years ago
|
||
For autocomplete brackets to work, you will have to do (or base this patch on top of ) something like what is done in attachment 8428318 [details] [diff] [review] for bug 964356
Assignee | ||
Comment 19•10 years ago
|
||
(In reply to Girish Sharma [:Optimizer] from comment #18)
> For autocomplete brackets to work, you will have to do (or base this patch
> on top of ) something like what is done in attachment 8428318 [details] [diff] [review]
> [diff] [review] for bug 964356
I see. Thanks.
Assignee | ||
Comment 20•10 years ago
|
||
This patch assumes bug 1029511 comment 4 is addressed.
Attachment #8446476 -
Attachment is obsolete: true
Comment 21•10 years ago
|
||
Comment on attachment 8445389 [details] [diff] [review]
reloadEditorPrefs.patch
Do not need this anymore after Bug 1031472. Now any opened editors will automatically reload after a pref changes
Attachment #8445389 -
Attachment is obsolete: true
Assignee | ||
Comment 22•10 years ago
|
||
Attachment #8446523 -
Attachment is obsolete: true
Attachment #8456005 -
Flags: review?(bgrinstead)
Comment 23•10 years ago
|
||
Comment on attachment 8456005 [details] [diff] [review]
v1
Review of attachment 8456005 [details] [diff] [review]:
-----------------------------------------------------------------
This is nice. The UI could use a bit of polish but it could mostly be tackled later, especially as more options get added.
::: browser/devtools/webide/content/prefs.js
@@ +18,5 @@
> +
> + // Buttons
> + document.querySelector("#close").onclick = CloseUI;
> + document.querySelector("#restoreButton").onclick = RestoreDefaults;
> + document.querySelector("#manageSimulators").onclick = () => window.parent.Cmds.showAddons();
Nit: I'd probably make a new function ManageSimulators or ShowAddons that moves the call to window.parent.* out of this function and to be consistent with CloseUI
::: browser/devtools/webide/content/prefs.xhtml
@@ +81,5 @@
> + </li>
> + <li>
> + <label><span>&prefs_options_tabsize;</span>
> + <select data-pref="devtools.editor.tabsize">
> + <option value="2">2</option>
Trailing whitespace
@@ +82,5 @@
> + <li>
> + <label><span>&prefs_options_tabsize;</span>
> + <select data-pref="devtools.editor.tabsize">
> + <option value="2">2</option>
> + <option value="4" selected="true">4</option>
Doesn't really matter one way or another, but this doesn't need to be selected since FillForm will set it before anything is seen
@@ +89,5 @@
> + </label>
> + </li>
> + </ul>
> +
> + <button id="manageSimulators">&prefs_simulators;</button>
I would suggest to move this to a link at the top next to the close link, similar to how the addons page has a 'open addons manager' nav link. But I'm fine either way
::: browser/devtools/webide/locales/en-US/webide.dtd
@@ +109,5 @@
> +<!ENTITY prefs_options_detectindentation "Detect indentation">
> +<!ENTITY prefs_options_detectindentation_tooltip "Guess indentation based on source content">
> +<!ENTITY prefs_options_autoclosebrackets "Autoclose brackets">
> +<!ENTITY prefs_options_autoclosebrackets_tooltip "Automatically insert closing brackets">
> +<!ENTITY prefs_options_expandtab "Expand tabs">
"Expand Tabs" is not very clear - maybe "Indent Using Spaces"?
::: browser/devtools/webide/themes/prefs.css
@@ +15,5 @@
> +body {
> + padding: 20px;
> +}
> +
> +h1 {
I'd suggest moving the common html/font/color/padding styles for this type of panel into a shared file - not a fan of maintaining duplicate CSS
Attachment #8456005 -
Flags: review?(bgrinstead) → review+
Assignee | ||
Comment 24•10 years ago
|
||
Attachment #8456005 -
Attachment is obsolete: true
Attachment #8456151 -
Flags: review+
Assignee | ||
Comment 25•10 years ago
|
||
Assignee | ||
Comment 26•10 years ago
|
||
Whiteboard: [fixed-in-fx-team]
Comment 27•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago → 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 33
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•