-
Notifications
You must be signed in to change notification settings - Fork 116
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
ENHANCEMENT Show state of elements draft/live/modified #374
ENHANCEMENT Show state of elements draft/live/modified #374
Conversation
e95d88a
to
5d35e78
Compare
5d35e78
to
bd486fa
Compare
bd486fa
to
65bfa37
Compare
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've made some suggestions. I'll add a commit to this PR with my suggested changes, feel free to blow them away if you don't like them though =)
'element-editor-header__version-state', | ||
{ | ||
'element-item--draft': !isPublished, | ||
'element-item--modified': isPublished && !isLiveVersion, |
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 think we should make the state a modifier on whatever the element already is. I think it'd be useful to apply this from the element-editor__element
element e.g. element-editor__element--modified
or element-editor__element--published
then propagate it down.
The CSS could then look like this:
.element-editor__element {
&--modified {
.element-editor-header__version-state {
// apply styles here for the circle
}
}
}
} | ||
|
||
if (isPublished && !isLiveVersion) { | ||
versionStateButtonTitle = 'Item has unpublished changes'; |
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 need to be translatable, and I think they could also be in their own method
@@ -32,5 +32,7 @@ Feature: Edit elements in the CMS | |||
And I press the "Publish" button | |||
Then I should see a "Published content block" message | |||
When I go to "/admin/pages/edit/show/6" | |||
Then I should see "Eve's Block" | |||
And I wait until I see the ".element-editor__element" element |
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 think I mentioned this on another PR - this assertion isn't designed to select a class - you're basically saying "wait until I see the [any element] element" where it's designed to be more specific than that e.g. "wait until I see the [specific] element".
We could add another step like "wait until the elements have loaded" instead maybe?
By the way, I realise re: the class names that you're re-using existing styles from the PHP GridField equivalent, but I think we should migrate them to the React structure and update the legacy reports GridField implementation to use those instead |
…s, make Behat look wait for elements to be rendered
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.
@raissanorth if you're happy with my new commit then merge at your leisure
Very happy indeed, so will leisurely merge ;) |
Styling based on current implementation:
Note that the CSS changes minimally increase the distance between the header title and the summary content.