45495 - gui for editing form elements

Status: RESOLVED FIXED

(SeaMonkey :: Composer, enhancement, P2)

Description

[Kathleen :Brade]

We need dialogs and related JS for editing form elements in a more gui dialog.

Comment 1

[rubydoo123]

adding help wanted keyword

Comment 2

[Kathleen :Brade]

(and of course we need menuitems or other ui to insert these as well)

Comment 3

[Ryan Cassin]

*** Bug 88017 has been marked as a duplicate of this bug. ***

Comment 4

[Kathleen :Brade]

*** Bug 90543 has been marked as a duplicate of this bug. ***

Comment 5

[Thomas Swan]

After adding a select, I noticed that the Editor had a hard time with the
<option>   tags.  I searched for this in bugzilla... Sorry for the dupe...

Comment 6

[[e-mail]]

How would go go about inserting a <form> anyway? "Convert selection to form"?
Have I forgotton any form elements:
form, input, textarea, select, option, optgroup, label, fieldset.

Comment 7

[Thomas Swan]

I did it while editing in the HTML view

Comment 8

[Syd Logan]

spam composer change

Comment 9

[[e-mail]]

Attachment: /mozilla/editor/ui/composer/locale/en-US/EditorInputProperties.dtd

Comment 10

[[e-mail]]

Attachment: /mozilla/editor/ui/dialogs/content/EdInputProps.js

Comment 11

[[e-mail]]

Attachment: /mozilla/editor/ui/dialogs/content/EdInputProps.xul

Comment 12

[[e-mail]]

Attachment: patch the 3 files in

Comment 13

[[e-mail]]

Attachment: new input and textarea dialogs

Comment 14

[[e-mail]]

Attachment: updated patches

Comment 15

[[e-mail]]

Attachment: revisions as per brade's comments

Comment 16

[[e-mail]]

Attachment: patches as per brade's comments

Comment 17

[[e-mail]]

Attachment: image input bugfix

Comment 18

[[e-mail]]

Attachment: image input bugfix patch

Comment 19

[Charles Manske]

All of this looks ok to me. Neil will be updating full patches with minor 
changes and I'll do final review then
Issues I see: 
1. I'm not sure if we really need 3 separate error
messages:
+InputNameError=You must enter a name for this type of form field.
+InputValueError=You must enter a name and value for this type of form field.
+TextAreaNameError=You must enter a name for this text area.
Maybe we can use just one message such as 
"You need a name or value for this form control."
to use for all 3 cases?
2. I'm not sure if we must do the "insert form" work as well before this is 
all checked in.

Comment 20

[[e-mail]]

Attachment: fix 5 minor issues

Comment 21

[Kathleen :Brade]

Here is what I'd like changed before we checkin:

** Change the strings from:
   You must enter a name for this type of form field.
to something like:
   Please enter a name for this form field.
** I think I'd like to see form fields completely collapsed/hidden rather than 
just disabled.  Charley--what do you think?
** Make "Field Value" the Image src for input type=image
   Eventually we should bring in src, alt, choose, and possibly relativize from a 
shared overlay; I would be ok on holding off on this step for now
** Remove the Image Properties... button
** Adjust More/Fewer items
   I'm leaning toward moving disabled, selected, size, length into the bottom 
portion.  Something like this:
  TYPE        TOP OPTIONS                  BOTTOM OPTIONS
  Text        Field Name (reqd)            Initial value   Disabled    ReadOnly
                                           Field Size      MaxLength   TabIndex
  Password    <same as text>

  Checkbox    Field Name (reqd)  Checked   Value           Disabled    TabIndex
  Radio       Group Name (reqd)  Checked   Value           Disabled    TabIndex

  Submit      Field Name (reqd)  Value     Disabled        TabIndex    AccessKey
  Reset       <same as submit>
  Button      <same as submit>

  File        Field Name (reqd)            Accept Types    Disabled    TabIndex
  Hidden      Field Name (reqd)            Initial Value   Disabled
  Image       Field Name (reqd)  src                       Disabled    TabIndex
              alt (include choose btn)

I hope the above is clear.
I think it'll make it smaller in the "fewer" case and be easier for novice users.

** For Textarea dialog, Rows and Columns are required
** For Textarea dialog, Wrap is not supported so it should be removed
** For Textarea dialog, I'd prefer to use more/fewer for some of these things 
rather than introducing a separate tab for default text:
     Top:  Field Name, Rows, Columns
     Bottom:  Disabled, ReadOnly, TabIndex, AccessKey, Default Text
** For Textarea dialog, try to be consistent with ordering in "more section" (for 
example, disabled comes above accesskey)

I hope the above doesn't seem too overwhelming.  I'm hoping the top portion can 
be "required" and commonly used while the "more" or bottom sections can be tucked 
away but still accessible if desired.  I'd even be ok with not offering those and 
just letting users access those things via Advanced Edit dialog.

Thanks for your work on these items!

Comment 22

[Christopher Aillon (sabbatical, not receiving bugmail)]

> Have I forgotton any form elements:
> form, input, textarea, select, option, optgroup, label, fieldset.

<button> and <legend>

Comment 23

[[e-mail]]

Wouldn't collapsing the input fields change the size of the window annoyingly?

I can add disabled, size and length to the "more" input fields.

I didn't realise that rows and columns were required for textareas.

How big do you want the default text field to be? Currently its size is automatic.

Comment 24

[[e-mail]]

Attachment: EdSelectProps.xul preview

Comment 25

[[e-mail]]

Attachment: Most of brade's requests

Comment 26

[[e-mail]]

Attachment: Patches for above

Comment 27

[[e-mail]]

Attachment: Includes <select>

Comment 28

[[e-mail]]

Attachment: patches for above

Comment 29

[[e-mail]]

Attachment: Includes <select>

Comment 30

[[e-mail]]

Attachment: Includes <select>

Comment 31

[[e-mail]]

Attachment: UI for <form>, <input>, <textarea>, <select>

Comment 32

[[e-mail]]

Attachment: patches for above

Comment 33

[[e-mail]]

Attachment: UI for <form>, <input>, <textarea>, <select>

Comment 34

[[e-mail]]

Forms and labels are currently invisible except in show all tags mode.
My preference would be for a 1px dotted green border, or blue for labels.

Comment 35

[[e-mail]]

Attachment: Special UI for image input

Comment 36

[[e-mail]]

Attachment: patches for above

Comment 37

[[e-mail]]

Attachment: Complete new UI

Comment 38

[[e-mail]]

Attachment: patches for above

Comment 39

[Charles Manske]

Neil: Your patch on 11/20 doesn't contain necessary changes to EdImageProps.xul.

Comment 40

[[e-mail]]

Attachment: patches for above

Sorry, I don't know how I remembered to diff the js and not the xul :-(

Comment 41

[[e-mail]]

Attachment: Grid changed code so bodged select dialog

Comment 42

[Asa Dotzler [:asa]]

Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)

Comment 43

[[e-mail]]

Attachment: Brade's tabbed form image dialog

Comment 44

[[e-mail]]

Attachment: patches for 60626

Comment 45

[[e-mail]]

Attachment: CManske's tabbed image dialogs

Comment 46

[[e-mail]]

Attachment: patches for 60800

Comment 47

[[e-mail]]

Attachment: Tabbed image dialogs, take 2

Comment 48

[[e-mail]]

Attachment: patches for 61718

Comment 49

[Charles Manske]

Comment on attachment 61718 [details]
Tabbed image dialogs, take 2

r=cmanske

Comment 50

[Charles Manske]

Comment on attachment 61720 [details] [diff] [review]
patches for 61718

r=cmanske

Comment 51

[Charles Manske]

We should get this checked in soon for testing. I will put the "Form" submenu
in the Debug Menu instead of "Insert" to emphasize that it is still a "work in
progress".
Known Issues:
1. Bug 82547: Clicking on form widgets steals the mouse events so we can't 
double-click to bring up Properties dialog.
2. Caret disappears after inserting an input element, type="text"
3. We need to show an icon in page for start and end of form and/or use some other 
CSS to show the are of the page inside the form.
To move this along, I think we should first checkin the changes to the Image 
dialog (redo layout and use a new overlay file to share with form dialog),
since we will keep those changes even if we are forced to postpone the insert
form UI.

Comment 52

[Charles Manske]

After discussing this further, we decided it's time to check this all in, but
put the "Form" submenu in the Debug menu untill all issues are resolved.

Comment 53

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 61720 [details] [diff] [review]
patches for 61718

sr=[e-mail]

Comment 54

[[e-mail]]

> 3. We need to show an icon in page for start and end of form and/or
> use some other CSS to show the are of the page inside the form.

Most other HTML editors display some sort of border like that for empty cells.

Comment 55

[Charles Manske]

12/14 patches checked in. Keeping bug open for further development.

Comment 56

[[e-mail]]

Attachment: patch to editor.properties

Some of the wrong strings went in.

Comment 57

[[e-mail]]

Attachment: put flex back on image preview box

I missed EdDialogOverlay from my patch so there were two boxes fighting for the
flex, which was wrong, but you weren't to know that the flex was supposed to be
on the image preview box and not the advanced edit box.

Comment 58

[Charles Manske]

Comment on attachment 65631 [details] [diff] [review]
patch to editor.properties

FormInputError
is used by EdInputProps.js, no?

Comment 59

[Charles Manske]

Comment on attachment 65632 [details] [diff] [review]
put flex back on image preview box

I don't like it that way!
When there's no image, the groupbox flexed to the right looks unbalanced. I did
that on purpose to let it expand only when the Actual Width and Height
data are displayed.

Comment 60

[Charles Manske]

Attachment: Fix for various dialog layout issues

What this fixes:
1. Includes changes to editor.properties from Neil in patch 65631
2. Changes to EdAEHTMLAttributes.js and EdHLineProps.js are replacements of 
".attributes.getNamedItem()" with new, more efficient "hasAttribute()"
3. Elimination of unnecessary groupboxes in Image properties panels for
"Location" and "Dimension". (Large amount of change is because of indentation 
changes.)
4. In EdInputProps.xul, the one grid encompassing the "MoreSection" was split
into 2 grids to solve problem of cut-off text labels in the "more" section.
Dialog now expands horizontally when "More/Fewer" is toggled, but I think that
is preferable to chopped off text.
The changes to EdImageProps.xul and EditorImageProperties.dtd include fix for
bug 114531, which adds radio buttons for the Alternate Text feature. It was 
impossible to separate without rendering the patch unapplyable.

Comment 61

[[e-mail]]

Comment on attachment 65656 [details] [diff] [review]
Fix for various dialog layout issues

I found an alternative to rewriting the grids: in EdDialogCommon.js instead of
using sizeToContent() use setTimeout(sizeToContent, 0)

Comment 62

[[e-mail]]

I've noticed that I'm calling document.createTextNode in a couple of places,
should that be editorShell.editorDocument.createTextNode instead?

Also, when inserting said text node the selection moves, should I put it back on
the original element?

Comment 63

[Charles Manske]

Just to be safe, I'd say you should always use 
editorShell.editorDocument.createTextNode.
I'm not sure if it really makes a difference in this case, but it is the 
document that you are going to insert into, so it's the right thing to do.

Comment 64

[Charles Manske]

Neil: I tested using "setTimeout(window.sizeToContent, 0)" and it didn't seem
to help the cropped labels in the "more" section of the Input Field dialog
(EdInputProps.xul,.js).

Comment 65

[[e-mail]]

Attachment: Fix text area inital value

This also includes the most reliable more/fewer changes for text areas, and
some extra changes to get the CSS white-space property for text area wrap.

Comment 66

[Charles Manske]

Comment on attachment 65631 [details] [diff] [review]
patch to editor.properties

r=cmanske

Comment 67

[Charles Manske]

Comment on attachment 65632 [details] [diff] [review]
put flex back on image preview box

I don't think we should do this one :)

Comment 68

[Charles Manske]

Comment on attachment 66101 [details] [diff] [review]
Fix text area inital value

r=cmanske

Comment 69

[[e-mail]]

Comment on attachment 65656 [details] [diff] [review]
Fix for various dialog layout issues

r=[e-mail]

Comment 70

[[e-mail]]

Attachment: Fix to use CSS caption-side in field sets

Note that layout doesn't actually appear to support align or caption-side :-)

Comment 71

[Charles Manske]

Comment on attachment 66283 [details] [diff] [review]
Fix to use CSS caption-side in field sets

r=cmanske

Comment 72

[Joe Hewitt (gone)]

Comment on attachment 65631 [details] [diff] [review]
patch to editor.properties

sr=hewitt

Comment 73

[Joe Hewitt (gone)]

Comment on attachment 65656 [details] [diff] [review]
Fix for various dialog layout issues

sr=hewitt

Comment 74

[Joe Hewitt (gone)]

Comment on attachment 66101 [details] [diff] [review]
Fix text area inital value

sr=hewitt

Comment 75

[Joe Hewitt (gone)]

Comment on attachment 66283 [details] [diff] [review]
Fix to use CSS caption-side in field sets

sr=hewitt

Comment 76

[Charles Manske]

Comment on attachment 65631 [details] [diff] [review]
patch to editor.properties

checked in

Comment 77

[Charles Manske]

Comment on attachment 65656 [details] [diff] [review]
Fix for various dialog layout issues

checked in

Comment 78

[Charles Manske]

Comment on attachment 66283 [details] [diff] [review]
Fix to use CSS caption-side in field sets

checked in

Comment 79

[[e-mail]]

Attachment: Make <form> and <label> visible

Comment 80

[Charles Manske]

I'd suggest another color for "label" color, since we use a solid blue line
to select images and other elements.

Uggh! Frustrating problems:
1. If you insert a form, you can't place the caret before or after it, so if it's
the first thing you do, you have forced the entire page to be a form!
3. After inserting a label, a single click in the label seems to select all the
label text, so you can't place caret in there to re-edit the text.

Comment 81

[Thomas Swan]

Regarding comment 80, that is a problem for tables as well.

Comment 82

[Charles Manske]

Kin: It's worse than the caret placement for tables problem -- that is fixed
by special code that does let you click before/after and type.

Comment 83

[[e-mail]]

OK, so would anyone like to volunteer a label colour?

Comment 84

[[e-mail]]

Attachment: Revised as per IRC discussion

Comment 85

[Charles Manske]

changing milestone

Comment 86

[Kathleen :Brade]

*** Bug 145352 has been marked as a duplicate of this bug. ***

Comment 87

[Charles Manske]

Form problem bugs 82547 and 162557 still hinder editing form elements in
Composer, but we are trying to get traction on those problems during 1.2.

Comment 88

[Syd Logan]

nsbeta1-, not a polish bug

Comment 89

[[e-mail]]

Attachment: Remaining issues

* Styles for <form> and <label>
* Access keys
* Improved code for adding/removing containers
* More try/catch
* Fix for error in <select> dialog
* Fix for text extraction in <label> and <legend>
* Fix for typos in <fieldset> dialog

Comment 90

[[e-mail]]

Comment on attachment 102565 [details] [diff] [review]
Remaining issues

MrJones1_2b points out (confirmed by brade) that the selection list properties
dialog doesn't work at all without this patch.

Comment 91

[[e-mail]]

Attachment: Fix blank text node issue and JS warnings

Comment 92

[Alec Flett]

Comment on attachment 109104 [details] [diff] [review]
Fix blank text node issue and JS warnings

this seems fine, and sr=alecf as long as the reviewer is charlie or another
editor person.

Comment 93

[Vidar Haarr (not reading bugmail)]

Does this patch cover what the author of 
<http://www.thesitewizard.com/reviews/mozillacomposer.shtml> considers one of 
the biggest problems of Composer ? See the last section of the 
review: "Verdict".

Comment 94

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Raising on radar. Forms should be in Composer, not in debug menu, for Mozilla 1.5.

Comment 95

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Comment on attachment 109104 [details] [diff] [review]
Fix blank text node issue and JS warnings

r=glazman
Check that in!

Comment 96

[[e-mail]]

Checked in the latest patch; waiting for the inevitable fallout :-)

Comment 97

[[e-mail]]

No fallout for once, I'm surprised, is my coding improving at last? ;-)