Skip to content

Consistently headline case inline action links #978

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

Merged
merged 1 commit into from
Dec 5, 2016

Conversation

spadgett
Copy link
Member

@spadgett spadgett commented Dec 2, 2016

We sometimes use headline case and sometimes sentence case for inline action links and links in alerts. Consistently use sentence headline case throughout the console.

Fixes #865
See also #805

@jwforres OK with this change? I don't have a strong opinion about headline vs sentence, but I'd like to be consistent.

@jwforres
Copy link
Member

jwforres commented Dec 2, 2016

@spadgett does the patternfly terminology doc specify? @ajacobs21e do you have a preference? this means "View Log" vs "View log"

@spadgett
Copy link
Member Author

spadgett commented Dec 2, 2016

Note that sometimes they're longer like "Don't show me again" or "Reload environment variables"

@spadgett
Copy link
Member Author

spadgett commented Dec 2, 2016

It's not clear to me these links fall into any of the categories on the Patternfly terminology page

http://www.patternfly.org/styles/terminology-and-wording

@ajacobs21e
Copy link

ajacobs21e commented Dec 2, 2016 via email

@spadgett
Copy link
Member Author

spadgett commented Dec 2, 2016

Should "Show advanced routing, build, deployment and source options" be headline as well?

@spadgett
Copy link
Member Author

spadgett commented Dec 2, 2016

Probably the longest one:

openshift_web_console

@jwforres
Copy link
Member

jwforres commented Dec 2, 2016

i think the ones that are more than about 3 words get terrible to read when in headline case

@ajacobs21e
Copy link

@spadgett @jwforres here are two examples of using Headline Case in PatternFly
https://blog.patternfly.org/lets-talk-more-about-labels/#Link
http://www.patternfly.org/pattern-library/forms-and-controls/expand-collapse-section/#/api
In most cases these links should be only a few words and Headline.

For the long example, we could edit the text or change it to a sentence (like the Links section here http://www.patternfly.org/styles/terminology-and-wording/)
Some options:

  • Show Advanced Options
  • Make a header for "Source, Routing, Build, and Deployment Options" with a link below "Show Advanced Options"
  • Show < Advanced options > for source, routing, build, and deployment settings.

@spadgett
Copy link
Member Author

spadgett commented Dec 5, 2016

@jwforres @ajacobs21e

openshift_web_console

@spadgett spadgett changed the title Consistently sentence case inline action links Consistently headline case inline action links Dec 5, 2016
@spadgett
Copy link
Member Author

spadgett commented Dec 5, 2016

Updated links to headline case.

@@ -162,7 +162,7 @@ angular.module('openshiftConsole')
!$filter('canI')('builds/log', 'get')) {
return [{
href: Navigate.resourceURL(build),
label: "View Build"
label: "View build"
Copy link
Member

Choose a reason for hiding this comment

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

I think you need to remove this change

@@ -153,11 +153,11 @@
<span ng-if="latestBuild | buildLogURL">
<!-- Always show a log link for pipeline builds. -->
<span ng-if="latestBuild | isJenkinsPipelineStrategy">
<a ng-href="{{latestBuild | buildLogURL}}" target="_blank">View Log</a>
<a ng-href="{{latestBuild | buildLogURL}}" target="_blank">View log</a>
Copy link
Member

Choose a reason for hiding this comment

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

revert this and the one right below

@@ -1 +1 @@
<div ng-if="(build | buildLogURL) && ('builds/log' | canI : 'get')" class="pipeline-link"><a ng-href="{{build | buildLogURL}}" target="_blank">View Log</a></div>
<div ng-if="(build | buildLogURL) && ('builds/log' | canI : 'get')" class="pipeline-link"><a ng-href="{{build | buildLogURL}}" target="_blank">View log</a></div>
Copy link
Member

Choose a reason for hiding this comment

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

revert

@spadgett
Copy link
Member Author

spadgett commented Dec 5, 2016

@jwforres thanks, updated

@jwforres
Copy link
Member

jwforres commented Dec 5, 2016

LGTM then, will let you decide how to order this and #985

@jwforres
Copy link
Member

jwforres commented Dec 5, 2016

just saw your comment in the other PR [merge]

@openshift-bot
Copy link

Evaluated for origin web console merge up to 3ac1443

@openshift-bot
Copy link

openshift-bot commented Dec 5, 2016

Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/802/) (Base Commit: 8bca523)

@openshift-bot openshift-bot merged commit 6cbd1a7 into openshift:master Dec 5, 2016
@spadgett spadgett deleted the inline-action-case branch December 5, 2016 17:56
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.

4 participants