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

Make UI more like other admin apps, prevent accidental publishing #464

Closed
wants to merge 15 commits into from

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Apr 2, 2015

  • Use Bootstrap form styles rather than custom ones (and extract common form parts into partials)
  • Use admin template pattern for attachments table, and truncate large tables with a JS toggle (some documents have over 100 attachments) – instead of alphagov/specialist-publisher#461
  • Separate publish, edit and withdraw actions and add confirm dialogues to irreversible actions
    • Completely hide publish button when unavailable, a disabled button is
      confusing
  • Correctly space sections on the show pages with a consistent vertical rhythm
  • Use lead styles to make summary more readable
  • Remove collapsible body function, instead restrict height, and don’t show at all unless there is a body
  • Reduce dependencies on govuk_frontend_toolkit – currently Bootstrap and the toolkit don’t play well together

https://www.pivotaltracker.com/story/show/90529348

Editing a document

Before

screen shot 2015-04-02 at 11 55 02

After

screen shot 2015-04-02 at 11 55 21

Viewing a document

Before

screen shot 2015-04-02 at 11 53 32

Actions:
screen shot 2015-04-02 at 11 53 46

After

screen shot 2015-04-02 at 11 53 58

Actions:
screen shot 2015-04-02 at 11 54 13

Truncated attachments table

screen shot 2015-04-02 at 12 09 22

Warnings

screen shot 2015-04-02 at 11 54 33
screen shot 2015-04-02 at 11 54 22

fofr added 15 commits Mar 27, 2015
Match the behaviour of other admin apps and use styles provided by the
admin gem.
* Remove “hover” style, items aren’t clickable, use striped instead
* Add a missing table row element
* Add table-header class to highlight headings
Pickup latest version of select2 library
Prevent checkboxes from appearing too far left of content. This matches
the recommended Bootstrap 3 pattern: http://getbootstrap.com/css/#forms
A row with col-md-12 in this case is the same as no row and no cell.
* Make it harder to accidentally click the wrong one.
* Put the more commonly used edit button first.
* Hide the publish button altogether when you can’t publish, disabled
buttons have proved to be confusing in the past and users continue to
try and interact with them
Use a lighter colour to indicate when an item was updated, to make the
list of sections or manuals easier to scan.
* Correctly space sections with a consistent vertical rhythm
* Use `lead` styles to make summary more readable
* Remove collapsible body function, instead restrict height, and don’t
show at all unless there is a body
* Remove dependencies on govuk_frontend_toolkit – currently the admin
template and the toolkit don’t play well together
A date is usually enough to determine how recently something has been
changed.
`<pre>` tags have a default word wrap that causes words to break
mid-word, which makes reading content difficult.
* Bump admin gem to pick up confirm JS module
* Use module to warn when clicking publish or withdraw buttons and to
confirm user action
* Separate publish, withdraw and edit buttons to avoid accidental clicks
* Always include a title on the “publish” container
* Completely hide publish button when unavailable, a disabled button is
confusing
* Some documents have a couple of attachments, others have hundreds
* Always show the first 5 attachments
* When more than 5 and when JS is enabled, hide the rest behind a
toggle with a link that indicates how many more attachments will be
revealed
* Pattern pulled from Transition where it was well received for tables
containing hundreds of mappings
@fofr fofr mentioned this pull request Apr 2, 2015
@russellthorn
Copy link
Contributor

@russellthorn russellthorn commented Apr 7, 2015

Looks good to me. It's probably worth getting a ruby dev to check the ruby, but otherwise I'm happy.

@evilstreak
Copy link
Contributor

@evilstreak evilstreak commented Apr 7, 2015

It looks like you've swapped a collapsing body for a fixed height display. How easy is it to scan through the content when it's shown in a small slice? Are editors looking at that content at all? If they're not, we could probably drop it. If they are, I worry that showing such a narrow slice will make scanning through long documents pretty impossible.

👍 for the truncated attachments table.

@@ -1,14 +1,18 @@
<div class="panel panel-danger">
<% if warning %>
<% unless disabled %>

This comment has been minimized.

@evilstreak

evilstreak Apr 7, 2015
Contributor

It seems weird to allow this partial to be included with a parameter that results in it being a no-op. Would it be better to just not include the partial?

@evilstreak
Copy link
Contributor

@evilstreak evilstreak commented Apr 7, 2015

In a couple of commits you hide the publish button altogether when the action is unavailable because "disabled buttons have proved to be confusing in the past". The advantage of showing a disabled button is that it distinguishes between the user not having permissions to publish, and the content item not being a suitable state to be published. Is that a valuable distinction to maintain? If so, is there another way to solve it without using disabled buttons?

@fofr
Copy link
Contributor Author

@fofr fofr commented Apr 7, 2015

@evilstreak The fixed body height is a technique used on other publishing tools on textareas when entering body text. If it's not quite right we can iterate. This is preferable to clicking open on an expand link and seeing an empty box.

@fofr
Copy link
Contributor Author

@fofr fofr commented Apr 9, 2015

To summarise the hangout I've had with @evilstreak, for this PR we are going to:

  • Move the withdraw button to a different place, because its actions are more like those of publish and it's not a safe action
  • Re-instate the warning messages for when something can't be published, and check the appropriateness of those messages

For a later PR and after some research:

  • Consider either remove the body excerpt altogether, or making it easier to expand and see all of the body copy if it's being used
@fofr
Copy link
Contributor Author

@fofr fofr commented Apr 9, 2015

Closing this PR until the first two points are finished.

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

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.