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

add unprose class, closes #793 #795

Merged
merged 4 commits into from
Jun 29, 2017
Merged

add unprose class, closes #793 #795

merged 4 commits into from
Jun 29, 2017

Conversation

samanpwbb
Copy link
Contributor

Unprose needs to be added to each element that you want to strip prose styles from, which could lead to a lot of uprose's all over the place. But I don't see a better way. Note that this PR also removes the line:

  text-decoration: none !important; /* prevent underline when inside .prose */

from toggles and buttons – instead we should recommend folks use .unprose to solve this problem.

@davidtheclark for review.

@samanpwbb samanpwbb requested a review from davidtheclark June 11, 2017 00:00
@@ -31,7 +31,7 @@
*
*/
.table,
.prose table {
.prose table:not(.unprose) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can create unprose containers, as well, by changing these selectors like so:

prose table:not(.unprose, .unprose table)

I think we should still recommend applying it directly to the elements that you're unprosing, because otherwise you could get yourself in a little bit of a confusing tangle about whether you're in a prose section or not. But do you think it's still worthwhile to add this capability?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoa the :not select doesn't work the way I thought it worked, but that makes sense. I prefer the idea of unprose containers, because it mirrors prose containers. Would you be opposed to using only prose table:not(.unprose table)?

Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 ... We'd need to make it clear that you can't nest these things (e.g. prose > unprose > prose > unprose). That's one advantage of applying unprose directly to elements: you can't mess it up. Other reason I lean that way is that I suspect this will mostly be used for single elements, right? You have a special link or header, but you still want everything around it to behave as prose. You disagree?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to either offer .unprose containers, or offer .unprose as a single-element modifier. Giving the class both qualities feels unnecessary. What do you think about leaving this PR as-is and merging?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm good with this (apply unprose directly to elements), as long as there's a little more explanation in the docs. However, if you really favor the container, I'm ok with you making the choice.

@davidtheclark
Copy link
Contributor

@samanpwbb: Before I update this branch and merge I just want to make sure we're on the same page, since it's been a little while. Are you also ok with the .unprose class working as it currently does?

@samanpwbb
Copy link
Contributor Author

@davidtheclark yep, current implementation is 👍

@davidtheclark davidtheclark merged commit 270d834 into dev-pages Jun 29, 2017
@davidtheclark davidtheclark deleted the unprose branch June 29, 2017 21:00
davidtheclark pushed a commit that referenced this pull request Jul 2, 2017
samanpwbb added a commit that referenced this pull request Jul 5, 2017
* adjust icon sizing, add icon-wrapper class, closes #763

* rename icon-wrapper to icon-inliner, other small fixes

* force txt-bold to be 1em line height, fixes line height offsets, closes #775

* rework size scale, closes #749

* Add limiter class, consistify scale even more

* only single-declaration utility classes should have important (#785)

* New copy of Open Sans (#786)

Reverts prior line-height change to bold weight.

Closes #775.

* reset abbr text-decoration, fixes txt-abbr rule (#788)

* Weight loss program (#787)

See changes in changelog.md

* Lighten gray-faint and add a line of caution to color documentation

* Reintroduce darken5 and lighten5 for backgrounds only

* Add icons option to build script (#794)

options.icons is an array of icon names. If this option is used, only icon names that match options.icons will be included in Assembly. Closes #782

* adds color and hover state to prose links (#798)

* Make uglify-js a real dependency

Closes #800.

* add unprose class, closes #793 (#795)

* add select--xs class to match btn--xs (#811)

* fix spin animation utility (#812)

closes #790

* prepare v0.14.0 (#813)
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.

2 participants