Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RDP Panel: packet editor sidebar #375

Merged
merged 2 commits into from
Mar 31, 2015

Conversation

rpl
Copy link
Member

@rpl rpl commented Mar 24, 2015

This branch introduces the "Packet Editor" sidebar in the RDP Panel.

NOTE: I've moved our custom events in a "devtools:" namespace to prevent potential collisions with existent events in 88b31d2.

The packet editor is currently based on ImmutableJS (immutable data structures) and
immstruct (cursor and change history abstractions over the immutable
data structures)

Features:

  • send RDP Packet
  • add new fields to the packet
  • clear packet (set the base "requestTypes" packet)
  • undo/redo editor states
  • click on label to expand nested data
  • row actions popup on double click on a value
    (edit value, remove field from parent, add new field
    in arrays or object values)

rdp_packet_editor

TODO:

  • refactor packet-editor.js React components
  • introduce tests for the RDP Panel and packet editor sidebar
  • evaluate "remove immstruct dependency" and/or "remove the cursor"
  • evaluate "move immutable state into the packet-editor.js"
    (e.g. to turn this kind of editor into a self-contained components simpler to integrate into other panels)
  • turn the "row actions" popup into something with a better UI

rpl added 2 commits March 24, 2015 15:48
currently based on ImmutableJS (immutable data structures) and
immstruct (cursor and change history abstractions over the immutable
data structures)

main features:

- send RDP Packet
- add new fields to the packet
- clear packet (set the base "requestTypes" packet)
- undo/redo editor states
- click on label to expand nested data
- row actions popup on double click on a value
  (edit value, remove field from parent, add new field
  in arrays or object values)
@janodvarko
Copy link
Member

Great stuff, my comments:

  1. The editor UX should be similar to what we have done in Firebug 2 for the Watch panel. See wiki: https://getfirebug.com/wiki/index.php/Watch_Side_Panel The panel shows how rows should be edited/appended/removed
  2. A "type" field can automatically offer (auto-complete) options from ObjectActor.requestTypes (ideally: request fields like actor id, etc. are filled in automatically)
  3. I want to refactor the inspector into an independent extension and not sure how hard is to keep the stuff synced. Luca, do you want me to merge now (even not finished) or you can rebase on top of the new extension?

Honza

@rpl
Copy link
Member Author

rpl commented Mar 31, 2015

@janodvarko

Completely agree on 1 and 2.

About 3:
I took a look to the independent extension repo to better plan how to bring this stuff there,
and I'd like to rework the other React components in the rdp-inspector repo to the new scenario first.
(e.g. build a "top toolbar" React component inside the inspector-panel, unify the components tree into a new root React component)

If it makes sense to you: we can merge this just as a preview of the tool (just in case it could be useful to send a custom RDP packet), if it's disabled it should not generate any conflicts with the external addon and we can remove all the code related to the RDP Panel once the external addon covers enough features.

@janodvarko
Copy link
Member

I took a look to the independent extension repo to better plan how to bring this stuff there,
and I'd like to rework the other React components in the rdp-inspector repo to the new scenario first.
(e.g. build a "top toolbar" React component inside the inspector-panel, unify the components tree into a new root React component)

Cool. Note that the toolbar is already there (I just did it a few moments ago). Feel free to clean up the code (please follow the same code style as you see it in existing modules, it should be also close to Add-on SDK style).

I'd like to continue working (porting) the side details pane. Hope, we want have too much conflicts with modifying the same code.

If it makes sense to you: we can merge this just as a preview of the tool (just in case it could be useful to send a custom RDP packet), if it's disabled it should not generate any conflicts with the external addon and we can remove all the code related to the RDP Panel once the external addon covers enough features.

OK

Honza

janodvarko added a commit that referenced this pull request Mar 31, 2015
@janodvarko janodvarko merged commit a95a6c9 into firebug:master Mar 31, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants