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

Sentence cased text everywhere #3166

Merged
merged 3 commits into from
May 18, 2018
Merged

Sentence cased text everywhere #3166

merged 3 commits into from
May 18, 2018

Conversation

rndstr
Copy link
Contributor

@rndstr rndstr commented May 10, 2018

Follows Weave Cloud's direction of sentence case on most things.

@rndstr rndstr requested review from bia and bowenli May 10, 2018 21:08
@bowenli
Copy link
Contributor

bowenli commented May 10, 2018

I think keeping the "by foo" makes sense since it describes the grouping.

@rndstr rndstr force-pushed the text-capitalization branch from c7be682 to 29cf78c Compare May 10, 2018 21:14
@bia
Copy link
Contributor

bia commented May 11, 2018

LmostlyGTM.

  • Some spacing issues become even more prominent in sentence case.
    screen shot 2018-05-11 at 12 04 53
    I talked with @fbarl about it - the simplest solution would be to add about 5px more css padding around that text.

  • Let's rename 'Containers-by-image' to 'Containers by image'?

I think keeping the "by foo" makes sense since it describes the grouping.

@rndstr
Copy link
Contributor Author

rndstr commented May 11, 2018

@bia IMO the main problem is the table header does not line up with the column content (on osx and linux). And the horizontal padding in the header is 2px vs 4px for the content. Lining the headers up with the columns and increasing the padding to 4px looks good to me.

The misalignment appears to be intentional because we don't know how wide the scrollbar is on different OS' so we just pick an avg of 8px. See also #3158

@rndstr rndstr force-pushed the text-capitalization branch 2 times, most recently from 977b3a1 to 3fc7c1e Compare May 11, 2018 18:41
@rndstr
Copy link
Contributor Author

rndstr commented May 11, 2018

@bia just fixed the alignment and adjusted the padding in #3169, this is how it's going to look
scope-table-header

@rndstr rndstr force-pushed the text-capitalization branch 2 times, most recently from a0e216b to 24d8726 Compare May 11, 2018 20:27
@bia
Copy link
Contributor

bia commented May 14, 2018

Cool, anyways that looks better. Maybe a little more space between the arrow and 'Processes'?

rndstr added 2 commits May 15, 2018 11:36
Follows Weave Cloud's direction of sentence case on most things.
@rndstr rndstr force-pushed the text-capitalization branch from 24d8726 to 68d798f Compare May 15, 2018 18:40
@rndstr rndstr merged commit f012c23 into master May 18, 2018
@rndstr rndstr deleted the text-capitalization branch May 18, 2018 00:30
lilic pushed a commit to lilic/scope that referenced this pull request Jul 25, 2018
* Sentence cased text everywhere

Follows Weave Cloud's direction of sentence case on most things.

* More space between sorter caret and label

* Use full topology name for table header
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.

3 participants