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

Improved UI for actions in task/supervisor table #7528

Merged
merged 12 commits into from
May 6, 2019

Conversation

shuqi7
Copy link
Contributor

@shuqi7 shuqi7 commented Apr 23, 2019

  • The original action column in supervisor/task table mixed the conception of information and action; it only open a JSON object in a new tab without prettifying it. It also gets harder to extend the features in the row if we want to add more in the future
    image

  • Now: improved UI which combines all the functions into a dialog, that is more functional and easier to extend.

  • It shows the JSON in a prettified format and allows users to copy and download the JSON, or view the raw JSON
    image
    image

@davidagee
Copy link

Thanks for the quick turnaround on this, @shuqi7. This is looking great!

A couple of UI comments, mockups here for reference:

  • Can we stack the icons and labels vertically in the left sidebar. I see in the diff you're already writing custom CSS for this so flex-direction: column should get you most of the way there.
  • Can we ensure the left nav icons are 20px20px
  • Left nav looks too wide
  • Offset the json text area from the top of the modal
  • Design explicitly does not use a button group for action buttons in the bottom left, they should be individual buttons with padding between them
  • Separator between left sidebar and content area is missing.
  • Adjust padding on footer area
  • For the base data grid, reduce width of "Actions" column and center-align the zoom icon.

@vogievetsky
Copy link
Contributor

A number of things from just playing with this:

  1. could you make it more aligned:

image

  1. Make sure you guard the actions correctly. For example "Kill" should only show up for running tasks.

  2. The log display is messed up notice that it is showing the \n instead of rendering it as a new line

image

Copy link
Contributor

@vogievetsky vogievetsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have a look at grid layouts

web-console/src/dialogs/table-action-dialog.scss Outdated Show resolved Hide resolved
web-console/src/views/tasks-view.tsx Show resolved Hide resolved
web-console/src/dialogs/table-action-dialog.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@vogievetsky vogievetsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor change request

@vogievetsky
Copy link
Contributor

Looking good. 👍

Copy link
Member

@clintropolis clintropolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I overall like this change, those tables were very busy, but I'm wondering if perhaps the action buttons, especially 'terminate' and 'suspend'/'resume', are maybe too hidden?

Maybe they would be suited to either be pulled out into their own buttons alongside the info panel, or in some sort of pop-up menu so the things people are likely to do are front and center? Maybe it's fine like this though? I'm not sure either way 🙃

@vogievetsky
Copy link
Contributor

Made a number of updates to revive this PR and rebased this branch on top of the Data loader (GUI) #7572 changes.

Some highlights:

  1. My new changes address feedback from @clintropolis above

image

  1. There is now UI for API to drop and reload data by interval #7439

image

image

  1. Some rewording should address New Router UI: No way to enable/disable datasources. confusing 'drop data' option. #7506

  2. Some styling fixed for the supervisor status UI (notice the added title also according to feedback from @davidagee )

image

Copy link
Member

@clintropolis clintropolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants