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

add an header for client / server labels in the packets list #59

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

rpl
Copy link
Member

@rpl rpl commented Jul 13, 2015

This PR introduces:

  • a minimum inspector window size (700 x 600), because the tabbars and toolbars will doesn't render correctly on smaller window sizes
  • a small header bar (with a thin bottom border and box shadows) in the packets list which shows who send the packet
  • extract header bar labels into locales
  • rename to PacketsParticipantsBar
  • proposed labels: Actors (Server) and Toolbox (Client)

rdpi-participants-bar-0

rdpi-participants-bar-1

@rpl
Copy link
Member Author

rpl commented Jul 13, 2015

@captainbrosset I've just pushed this small tweak to the UI, how it looks to you?

@captainbrosset
Copy link

Cool, I think that helps.
2 things that come to mind, but I'm afraid I don't have solutions to propose:

  • visually speaking, maybe this needs to be more "integrated" in the UI somehow, maybe by adding a bottom border or something
  • about the labels: "server" and "client", there are other ways to call these things, and I'm afraid I don't know which is better. Client/Server is good because it's a model people can relate to. Here are other ways we've called them:
    • the server is the debuggee, or simply the target tab or device
    • the client is the debugger, or toolbox

rpl added 3 commits July 17, 2015 14:17
- extract labels into locales
- rename to PacketsParticipantsBar
- add bottom border and box shadow to the header bar
- change proposed labels to: Actors (Server) and Toolbox (Client)
@rpl rpl force-pushed the tweaks/packets-destinations-bar branch from 1cb5bd2 to cce12b6 Compare July 17, 2015 13:43
@rpl
Copy link
Member Author

rpl commented Jul 17, 2015

@captainbrosset @janodvarko I've just updated this branch with the following changes:

  • rebase to the new master branch
  • extract labels into locales
  • rename to PacketsParticipantsBar
  • add bottom border and box shadow to the header bar
  • change proposed labels to: Actors (Server) and Toolbox (Client)

@rpl
Copy link
Member Author

rpl commented Jul 17, 2015

@captainbrosset @janodvarko the screenshots in the description show the last changes

@janodvarko
Copy link
Member

Thanks for the work Luca!

(sorry for the delay, back from vacation today)

I share the concern related to "client/server" vs. "actors/toolbox" terminology and I am afraid that none of the suggestions makes the UI better.

The original idea behind the packet-list UI/UX has been mimicking well known user-chat UI (e.g. WhatsApp, SMS clients, etc.):

  • Incoming messages (packets) displayed on the left
  • Outgoing messages (sent by 'me') displayed on the right

Where 'me' (in our case) doesn't necessarily have to be the Toolbox. It can be any client that has a connection to the backend (or server or actors, huh).

I don't know how to make this basic UX concept better to understand, but note that:

  • every received message says: Received from <actor-name>
  • and every sent message says: Sent <message-type> to <actor-name>

Isn't that already enough indication of what is sent and what received?
Or could we rather improve the message (packet) representations itself?

We could yet change the labels to: "Received" and "Sent", but I would rather vote to not append the top bar at all.

Honza

@rpl
Copy link
Member Author

rpl commented Aug 3, 2015

@janodvarko No problem, I completely understand, and this is just a proposal, if we are unsure about how to change the UI, it is definitely better to not change it until we have a better idea.

I'm going to open another pull request and cherrypick 99c6f9d which is not related to this change and fixes a visual issue on the TreeEditorView component (the remove item button disappears when the mouse is over it).

@janodvarko
Copy link
Member

@janodvarko No problem, I completely understand, and this is just a proposal, if we are unsure about how to change the UI, it is definitely better to not change it until we have a better idea.

Yes

I'm going to open another pull request and cherrypick 99c6f9d which is not related to this change and fixes a visual issue on the TreeEditorView component (the remove item button disappears when the mouse is over it).

Excellent (also see my comment in the commit)

Honza

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