Bug 21108 - Impossible to add an attribute to a node without attributes
Summary: Impossible to add an attribute to a node without attributes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-09-25 10:05 PDT by Anthony Ricaud
Modified: 2012-07-26 21:44 PDT (History)
7 users (show)

See Also:


Attachments
Add Attributes to Nodes (6.16 KB, patch)
2009-07-21 16:36 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
Video of Usage (35 bytes, text/plain)
2009-07-23 09:04 PDT, Joseph Pecoraro
no flags Details
Add Attributes to Nodes (6.24 KB, patch)
2009-07-23 09:33 PDT, Joseph Pecoraro
timothy: review-
Details | Formatted Diff | Diff
Add Attributes to Nodes Improved (6.86 KB, patch)
2009-07-23 12:47 PDT, Joseph Pecoraro
timothy: review-
Details | Formatted Diff | Diff
Add Attributes to Nodes More Improved (6.86 KB, patch)
2009-07-23 13:22 PDT, Joseph Pecoraro
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anthony Ricaud 2008-09-25 10:05:54 PDT
Double clicking a node without attributes scopes the view. There's no way to easily add new attributes to a node.
Comment 1 Joseph Pecoraro 2009-07-21 16:36:58 PDT
Created attachment 33226 [details]
Add Attributes to Nodes

This allows users to add attributes to nodes from the Elements Panel in the Tree.

NOTES: 

1. Only nodes that are of type Node.ELEMENT_NODE will exhibit the behavior mentioned below, and only in the Elements Panel, not in the console when inspecting a node.
2. When hovering over the Node in the Tree view an ellipsis (…) will appear to the right of the text on the hovered line
3. Double Clicking the ellipsis (I didn't both with a single click at the moment) will trigger the usual dblclick
4. This notices it is inside the 'add-attribute' ellipsis and will dynamically create an attribute to start editing, like normal.

Give it a whirl.  Make suggestions on how to improve the User Interface to a tool like this.  Sometimes I really wish I had a designer to work with.
Comment 2 Anthony Ricaud 2009-07-22 06:30:41 PDT
Great !

Two comments :
Make the ellipsis more button-like to help understand that it is clickable.

Add a space before the editable zone to show that it's different from the previous attribute.

Otherwise it's looking fine and it's a nice addition to the inspector.
Comment 3 Timothy Hatcher 2009-07-22 11:55:14 PDT
Please attach a screenshot.
Comment 4 Joseph Pecoraro 2009-07-23 09:04:15 PDT
Created attachment 33337 [details]
Video of Usage

Maybe a video would have been better so I gave it a shot.  Please forgive my improper use of terminology in the video. Doctypes, Text, and Comments are their own NODE Types (DOCUMENT_TYPE_NODE, TEXT_NODE, and COMMENT_NODE respectively). The ellipsis should only show up for element nodes (ELEMENT_NODE).
http://screencast.com/t/PMZy8R419pD
Comment 5 Joseph Pecoraro 2009-07-23 09:33:49 PDT
Created attachment 33339 [details]
Add Attributes to Nodes

This has slightly better margins.  When you add a new attribute the textfield is no longer overlapping text from the attribute/text before it.  This was because the .editing class was giving it a negative margin.  I overrule that with a direct style property for the new attribute.
Comment 6 Timothy Hatcher 2009-07-23 10:33:35 PDT
Comment on attachment 33339 [details]
Add Attributes to Nodes

Thanks for the video! It helped a lot.

I wonder if a icon (button looking) would be better than the text ellipsis. Or move the ellipsis to the inside of the tag so it is where the new attribute will be insterted? What do you think?

> +            var span = document.createElement('span');
> +            span.className = 'add-attribute';
> +            span.innerText = '…';
> +            this.listItemElement.appendChild(span);
> +            this._addAttributeElement = span;

Better to use a escaped character (\uXXXX) instead of UTF8 here. Also we use double quotes for string almost esclusivly in our JS.

The textContent property is better to use than innerText. It has no IE weirdness and is standard.

> +                this._addAttributeElement.parentNode.removeChild( this._addAttributeElement );

No spaces needed here.

> +        if ( tag.getElementsByClassName('webkit-html-attribute').length > 0 ) {

No spaces needed here.

> +        return this._startEditingAttribute(attr, { target: attr });

No spaces needed here.
Comment 7 Timothy Hatcher 2009-07-23 10:34:44 PDT
> +        if ( tag.getElementsByClassName('webkit-html-attribute').length > 0 ) {

I mean no spaces needed here next to the parenthesis.
Comment 8 Joseph Pecoraro 2009-07-23 12:47:45 PDT
Created attachment 33355 [details]
Add Attributes to Nodes Improved

NOTES:

- I kept it a "..." but now everything else is out of the way so it will be easy to experiment with this.
- Moved the "..." inside the tag.
- Refactored to add _insertInLastAttributePosition function.
- Still double-click, without cursor: pointer
Comment 9 Timothy Hatcher 2009-07-23 12:55:14 PDT
Comment on attachment 33355 [details]
Add Attributes to Nodes Improved


> +            var nodeName = tag.textContent.match(/^<(.*?)>$/)[1];
> +            tag.textContent = '';
> +            tag.appendChild(document.createTextNode('<'+nodeName));
> +            tag.appendChild(node);
> +            tag.appendChild(document.createTextNode('>'));

Double quote strings and spaces around "+".
Comment 10 Joseph Pecoraro 2009-07-23 13:22:39 PDT
Created attachment 33360 [details]
Add Attributes to Nodes More Improved

Fixes those styles issues and the ChangeLog.
Comment 11 Adam Barth 2009-07-24 01:33:12 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/inspector/front-end/ElementsTreeOutline.js
	M	WebCore/inspector/front-end/inspector.css
Committed r46339
	M	WebCore/ChangeLog
	M	WebCore/inspector/front-end/ElementsTreeOutline.js
	M	WebCore/inspector/front-end/inspector.css
r46339 = 8f096edd27f484633afdef0d31ec1e2b7dc5e327 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46339