-
Notifications
You must be signed in to change notification settings - Fork 4
Changelog and entity-metadata layout (CFM-476) #349
Changelog and entity-metadata layout (CFM-476) #349
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| msgid "changelog.aria.collapsed" | ||
| msgstr "Change Log, collapsed, button, click to expand" | ||
|
|
||
| msgid "changelog.aria.expanded" | ||
| msgstr "Change Log, expanded, button, click to collapse" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per the documentation, the term is "changelog" not "change log". But what is this actually used for, is this an accessibility description?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an accessibility description - but a screen reader should read it correctly as both "changelog" and "change log" -- I'll make it the former so the name is consistent with the rest of the application.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes made.
| msgid "changelog.deleted" | ||
| msgstr "This record has been deleted" | ||
|
|
||
| msgid "changelog.return" | ||
| msgstr "Return to parent record" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be "active" not "parent" (which is the term used for TreeBehavior enabled models).
The documentation was inconsistent on this point, but I just cleaned it up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this key probably belongs in operation.po.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent - I was wondering about "parent" (since revisions don't imply such a relationship) - "active" is great.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes made.
| msgid "entity.created" | ||
| msgstr "Created: {0}" | ||
|
|
||
| msgid "entity.id" | ||
| msgstr "ID: {0}" | ||
|
|
||
| msgid "entity.modified" | ||
| msgstr "Modified: {0}" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These (including the existing id key) belong in fields.po, which already have entries for the same field names, so we probably need to call them created.changelog or something like that. (I'm not sure what a good term for this context is, "metadata" doesn't seem helpful, "lightbox" isn't right, etc...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These have been moved into field.po as
created.value
id.value
modified.value
This places each key next to it's plain-label counterpart.
app/templates/element/changelog.php
Outdated
|
|
||
| if(empty($vv_obj)) | ||
| return; | ||
| // If this is an archived record, include a link to the parent (active) record |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize this is my comment originally, but we should align it with the "correct" terminology...
// If this is an archive record, include a link to the active record
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
app/templates/element/changelog.php
Outdated
| ['action' => 'edit', $vv_obj->$clAttr] | ||
| ); | ||
| } | ||
| // Get the change log open/closed state |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No space in "changelog"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
app/templates/element/changelog.php
Outdated
| // We'll render an index of all archived records if we are the current active record, | ||
| // or just the current metadata if we are an archived record |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should fix my comment here too
// We'll render an index of all archive records if we are the current active record,
// or just the current metadata if we are an archive record
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. I've also changed the few other places where this was inconsistent (e.g. "View Change Log" is now "View Changelog".
ed3130c
to
1280a79
Compare
This PR updates the layout of the changelog, placing it in an accessible accordion that maintains state when opened or closed. This also allows us to move the entity metadata ID, created date, and modified date to a new region above the changelog.
The changelog for a parent record with revisions will look like this:

An archived (or deleted) record will provide a more visible notice of its status and a link back to the parent. It will look like this:
