Rename top-level menu to Batches #99

Merged
merged 4 commits into from Jan 11, 2017

Projects

None yet

3 participants

@jjeaton
Contributor
jjeaton commented Jan 10, 2017

Locomotive is fairly meaningless in the admin, this should
help users find the menu more easily.

Renamed the admin page title to 'Batch Processes' and made the strings
translatable.

Would love some other thoughts here

@jjeaton jjeaton Rename top-level menu to Batches
Locomotive is fairly meaningless in the admin, this should
help users find the menu more easily.

Renamed the admin page title to 'Batch Processes'
e127c4f
@norcross
Contributor

does this include a menu icon as well?

@jjeaton
Contributor
jjeaton commented Jan 10, 2017

@norcross It doesn't, that could be a separate PR.

@norcross
Contributor

sounds good. i'll do some digging there.

@JasonHoffmann

LGTM. At some point we may want to introduce a hook for someone to customize the name.

@jjeaton
Contributor
jjeaton commented Jan 10, 2017

This also makes me think whether we really need a top level menu item, or if we should move this into the Tools menu... We wouldn't need a menu icon then, and also are not adding to the glut of top level items. The only real reason to have a top level menu is Marketing! ☄️ or if you need submenu items. I'm not sure we'd ever need submenu items here.

@JasonHoffmann
Contributor

I can't imagine a day when this is more then just one screen. In which case you really don't need it's own menu item. Do we think the Marketing! ☄️ value is worth it? For developers installing it on clients sites, they would probably want it tucked away a bit right?

@jjeaton
Contributor
jjeaton commented Jan 10, 2017

I'd be OK with moving it to Tools menu.

@jjeaton jjeaton referenced this pull request Jan 11, 2017
Closed

admin menu icon #77

@jjeaton
Contributor
jjeaton commented Jan 11, 2017

It's agreed then, we're moving this to the Tools menu. I can come back to this, but if anyone wants to take it in the meantime, feel free.

@JasonHoffmann
Contributor

Sure, same PR or different one?

@jjeaton
Contributor
jjeaton commented Jan 11, 2017
@JasonHoffmann
Contributor

Cool, I'll add it.

@jjeaton
Contributor
jjeaton commented Jan 11, 2017

@JasonHoffmann tests are failing. We need this fix applied to our repo as well: reaktivstudios/civil-comments#5

@JasonHoffmann
Contributor

I'll dig in.

@JasonHoffmann
Contributor

Tests have been fixed

@jjeaton jjeaton merged commit 9af117f into master Jan 11, 2017

1 check passed

Scrutinizer Analysis: No new issues – Tests: passed
Details
@jjeaton jjeaton deleted the menu-title branch Jan 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment