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

NEW: Task runner better UI #301

Merged
merged 10 commits into from
Aug 26, 2020

Conversation

mfendeksilverstripe
Copy link
Contributor

@mfendeksilverstripe mfendeksilverstripe commented Jun 8, 2020

Task runner better UI

Simple UI changes which improve usability of the task runner. Lightweight CSS (no build chain or JS required).

Problem

  • it takes time to find the dev task you want to run especially if you don't know the exact task name
  • links which execute dev tasks are tiny, mis-click error can happen
  • current UI is cluttered with too many dev tasks and the space is used inefficiently
  • rendering component of the task runner is messy (combines backend and frontend logic)
  • overriding frontend display requires subclassing

Current UI

Screen Shot 2020-06-08 at 12 05 05 PM

Solution

  • move the task groups into separate tabs so the screen is less cluttered
  • visually separate the dev tasks
  • visually separate the action buttons and make them much more visible
  • split the rendering into backend (populate data) and a template (render), this allows the template to be overridden without subclassing

Proposed UI

Screen Shot 2020-06-08 at 1 16 11 PM

Related issues

#302

Dependencies

silverstripe/silverstripe-framework#9540

@mfendeksilverstripe mfendeksilverstripe force-pushed the feature/task-runner-ui branch 4 times, most recently from a821602 to e21b57f Compare June 8, 2020 02:53
.editorconfig Outdated Show resolved Hide resolved
@michalkleiner
Copy link
Collaborator

Very nice touch! Often these sparingly used areas of the CMS/admin interface don't get any updates/tweaks, so this is refreshing to see.

I'd suggest looking at it at the framework level though, if possible, where it lives without the queued jobs additions (run now/enqueue). Here it can subsequently alter the actions.

@mfendeksilverstripe
Copy link
Contributor Author

Will have a look at the core task runner. Thanks for feedback @michalkleiner

@mfendeksilverstripe
Copy link
Contributor Author

@michalkleiner Moved some of the code to the framework, please review.

Copy link

@sachajudd sachajudd left a comment

Choose a reason for hiding this comment

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

Review comments for framework can be found here but also apply to this PR: silverstripe/silverstripe-framework#9540 (review)

@mfendeksilverstripe
Copy link
Contributor Author

@sachajudd Pushed up new changes based on UI feedback, please review. This is how the UI looks like now:

Large screen

Screen Shot 2020-07-17 at 11 37 53 AM

Small screen

Screen Shot 2020-07-17 at 11 38 22 AM

@sachajudd
Copy link

See comments on silverstripe/silverstripe-framework#9540 (comment) which apply to this PR 🙂

@mfendeksilverstripe
Copy link
Contributor Author

New changes:

  • border hover colour fixed
  • more space for bottom border

@mfendeksilverstripe
Copy link
Contributor Author

New changes:

  • updates labels in other tabs.

This is now ready for a review.

Copy link

@sachajudd sachajudd left a comment

Choose a reason for hiding this comment

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

Approved from a UX perspective 🙂

Copy link

@Cheddam Cheddam left a comment

Choose a reason for hiding this comment

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

Hey Mojmir! This PR, along with the core one, is a great step forward for the dev/tasks UI. I've suggested some fairly minor modifications.

I'm also pondering whether it'd be sensible to reduce the code duplication between the base TaskRunner and this one by splitting some of the functionality out into separate methods / config? For example, the stylesheets could be stored in configuration, and the logic for handling them could be split into a method on the base TaskRunner class. This might be overkill, but it might also serve to reduce the amount of double-handling required in future updates to this UI (for example, I'm keen to introduce proper categorisation of tasks later on, along with a search field.)

Let me know what you think.

client/styles/task-runner.css Outdated Show resolved Hide resolved
client/styles/task-runner.css Outdated Show resolved Hide resolved
@mfendeksilverstripe
Copy link
Contributor Author

mfendeksilverstripe commented Jul 31, 2020

@Cheddam Pushed up changes as requested, please review.

Copy link

@Cheddam Cheddam left a comment

Choose a reason for hiding this comment

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

Excellent work, thanks for your contribution!

Copy link

@Cheddam Cheddam left a comment

Choose a reason for hiding this comment

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

Actually, one last minor tweak is required 😅

Since we standardised the way the CSS is included in the framework PR, this PR now needs to bump the minimum framework requirement to ^4.7 (the next minor release). I'll tweak this now.

@Cheddam Cheddam force-pushed the feature/task-runner-ui branch 2 times, most recently from 9a49e08 to 4273f6d Compare August 25, 2020 23:46
@Cheddam Cheddam changed the base branch from 4.5 to 4 August 25, 2020 23:58
Copy link

@Cheddam Cheddam left a comment

Choose a reason for hiding this comment

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

Okay, I've also rebased onto the 4 branch since this is a new feature, and fixed up Travis config. Once the tests go green we're good to merge 👍

@Cheddam Cheddam merged commit 5175489 into symbiote:4 Aug 26, 2020
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.

5 participants