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
Feature #9592: CSS Dropcaps, v6 #9609
Conversation
| @@ -38,6 +38,29 @@ $subheader-margin-bottom: 0.5rem !default; | |||
| /// @type Number | |||
| $stat-font-size: 2.5rem !default; | |||
|
|
|||
| /// @dropcaps | |||
|
|
|||
| // Line height for all the text within dropcaps, disabling default line height's | |||
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.
/// Comments are used to generate documentation with Sassdoc. For settings, a simple docs is like:
/// Default background color. <-- Formulated like "[This is the] Default backg..."
/// @type Color
$...-background: $white !default;I would also advise you to read http://sassdoc.com/annotations/.
| $dropcaps-first-letter-line-height: 0.85em !default; | ||
|
|
||
| // Dropcaps float | ||
| $dropcaps-first-letter-float: left; |
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.
We can only use $global-left without options. There is no case where you would want to have the first letter on the right with a LTR context.
| line-height: $dropcaps-first-letter-line-height; | ||
| float: $dropcaps-first-letter-float; | ||
| } | ||
| } |
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.
Please create a Sass component with a mixin:
@mixin dropcaps() { // we have no options for now, unless you want to provide `$dropcaps-lines` or `$dropcaps-space` now.
...code...
}Then use it to create the CSS component:
.dropcaps {
@include dropcaps;
}| $dropcaps-first-letter-margin-bottom: -0.05em !default; | ||
|
|
||
| // Padding Right to the dropcaps first letter to create gap b/w rest of text | ||
| $dropcaps-first-letter-padding-right: 0.05em !default; |
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.
You do not have to provide so much options. Components should be customizable, yes, but it is useless when options are magic numbers that folks cannot easily understand.
I made some search about how dropcaps in CSS are made and found this: http://code.stephenmorley.org/html-and-css/creating-drop-caps/
For now, we can assume these values will not change:
font-size: 5.2em;
line-height: .5;
margin: 0.192em 0.0962em 0 0;Later, we can add understable options like:
$dropcaps-lines: Number of lines the height of the first letter should be.$dropcaps-space: Both vertical and horizontal space between the first letter and the paragraph.
But it would require some maths.
| font-size: $dropcaps-first-letter-font-size; | ||
| line-height: $dropcaps-first-letter-line-height; | ||
| float: $dropcaps-first-letter-float; | ||
| } |
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.
You can add: text-transform: uppercase; to prevent problematic letters like q or p.
|
@ncoden .... No issue buddy .... Will try again |
|
@IamManchanda Please use semantic names for commit/issues/PRs. It should describe the changes you made (with a complete description for commits), and not what you personally done. ✅ Good:
❌ Bad:
|
|
so is there any git command to update my commit message ? @ncoden |
2b545c1
to
f2371d3
Compare
|
done @ncoden .... i have updated the commit message .... did it myself .... thanks to some googling and that git rebase practice at my first PR by kevin sir |
| @mixin foundation-dropcaps() { | ||
| font-size: $dropcaps-font-size; | ||
| line-height: $dropcaps-line-height; | ||
| vertical-align: $dropcaps-vertical-align; |
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.
vertical-align is useless when we use float, because the element is "out of the flow" and its position cannot be compared with the others component's one.
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.
ohk but i found this in your codepen so used it 😈
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.
Sorry about that 😄
| text-transform: $dropcaps-text-transform; | ||
| margin: $dropcaps-margin; | ||
| float: #{$global-left}; | ||
| } |
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.
Please use the Specific property order.
|
|
||
| // Use to create an initial aka dropcap | ||
| .dropcaps{ | ||
| line-height: 1.5; // Disabling default line height of an 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.
Why ???
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.
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.
No, keep it.
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.
So to expand on this & the result... this means that @include dropcaps or adding the .dropcaps class will change the line height when applied to tags like p or h1... ie my leading paragraph where I want to include a dropcaps will look fundamentally different than my other paragraphs...
And further, this is not accessible to change via a var or mixin parameter.
I'm not sure this is the right solution. What if we made this a parameter to the mixin and computed the :first-letter lineheight from it?
| line-height: 1.5; // Disabling default line height of an element | ||
| &:first-letter { | ||
| @include foundation-dropcaps; | ||
| } |
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.
The "dropcaps" component is the full paragraph. @mixin foundation-dropcaps should contain &:first-letter {...}.
| } | ||
|
|
||
|
|
||
|
|
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.
Empty lines here
|
|
||
| /// Deafult margin for dropcaps | ||
| /// @type Numbers | ||
| $dropcaps-margin: 0.192em 0.0962em 0 0 !default; |
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.
Like I said before, this is useless to provide it as a setting for now, because it is complex numbers the user cannot modify properly. We can provide a $dropcaps-margin option only if $dropcaps-margin: 0 gives the render we would expect by its name: no margins between the letter and the paragraph.
Please remove these settings and use private variables inside @mixin foundation-dropcaps.
|
|
||
| /// Default text transform for dropcaps | ||
| /// @type String | ||
| $dropcaps-text-transform: uppercase !default; |
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.
The only text-transform values that could make sense in this component are:
- inherit
- uppercase
So we should rather provide a $dropcaps-uppercase: true Boolean here.
|
|
||
| /// Default line height for dropcaps | ||
| /// @type Number | ||
| $dropcaps-line-height: 0.5 !default; |
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.
For $dropcaps-font-size and $dropcaps-line-height: private variables, see $dropcaps-margin below.
|
Try to use title for commits of less than 80 characters, and use the description for complete information. Sorry for all these remarks, I am very picky just for you so you can improve youself 😄 . |
|
No its ohkkk .... i dont mind .... these remarks is surely helping me out |
|
Confused about private variables ? @ncoden @kball Are you saying me to use it like this below or something else @mixin foundation-dropcaps() {
line-height: 1.5; // Disabling default line height of an element
&:first-letter {
float: #{$global-left};
margin: 0.192em 0.0962em 0 0;
font-size: 5.2em;
line-height: .5;
@if $dropcaps-uppercase {
text-transform: uppercase;
}
}
} |
/// Config variable
$config-variable: ...;
/// Component
@mixin component($parameter: ...) {
$private-variable: ...;
}A variable inside a component, that cannot be modified by user, separated from properties only for clarity. |
|
Oh You mean a global variable @ncoden Are you talking about this ? @mixin dropcaps($dropcaps-margin, $dropcaps-font-size, $dropcaps-line-height) {
line-height: 1.5; // Disabling default line height of an element
&:first-letter {
float: #{$global-left};
margin: $dropcaps-margin;
font-size: $dropcaps-font-size;
line-height: $dropcaps-line-height;
@if $dropcaps-uppercase {
text-transform: uppercase;
}
}
}& the CSS component .dropcaps{
@include dropcaps(0.192em 0.0962em 0 0, 5.2em, .5);
}Update after below review: changed foundation-dropcaps to dropcaps |
| /// @type Numbers | ||
| $dropcaps-margin: 0.192em 0.0962em 0 0 !default; | ||
|
|
||
| @mixin foundation-dropcaps() { |
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.
foundation-* mixins are only used to generate default styles. For now, component mixins does not have a prefix (but I agree they should have one).
So please use just dropcaps.
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.
ohk no issue for this one
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.
Infact changed , But tell me about the above one @ncoden
|
No, I mean a variable inside the mixin the user cannot modify: // Dropcaps component
@mixin dropcaps(
// -> Do not use global settings directly but via parameters, so they can be modified locally.
$uppercase: $dropcaps-uppercase
) {
// { comment to explain where these values come from }
// ...
$-margin: 0.192em 0.0962em 0 0;
$-font-size: 5.2em;
$-line-height: .5;
line-height: 1.5; // Disabling default line height of an element
&:first-letter {
float: #{$global-left};
margin: $-margin;
font-size: $-font-size;
line-height: $-line-height;
@if $dropcaps-uppercase {
text-transform: uppercase;
}
}
}
...
// Generate default styles
.dropcaps {
@include dropcaps();
}So later you can do: .my-own-dropcaps {
@include dropcaps($uppercase: false);
}But it would make no sense to do something like this: .my-own-dropcaps {
// -> Do not use
@include dropcaps($dropcaps-margin: 0 0 1em 0);
}... Because |
|
@IamManchanda Don't worry, I receive notifications ;) |
|
Check this out now nico bro /// Dropcaps component
@mixin dropcaps($uppercase: $dropcaps-uppercase) {
// Default margin from top and right to let it properly aligned
// and have a tiny horizontal space with rest of the text
$-margin: 0.192em 0.0962em 0 0;
$-font-size: 5.2em; // Default font size for dropcaps
$-line-height: .5; // Default line height for dropcaps
line-height: 1.5; // Disabling default line height of an element
&:first-letter {
float: #{$global-left};
margin: $-margin;
font-size: $-font-size;
line-height: $-line-height;
@if $dropcaps-uppercase {
text-transform: uppercase;
}
}
}But, i must say ..... from all of this ..... neither output or css is changing But hey, |
|
and hey how do you get these colored syntax ? @ncoden Update: No Thanks, i found it out 😉 |
|
Please write commit instead of writing text in comment, so I can review it line per line. And these |
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.
This is almost good :)
Now I have two questions for @kball:
- What do you think ?
- Should we move
@mixin-dropcapsto its own file like@mixin-header?
|
|
||
|
|
||
|
|
||
| } |
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.
EOL at the end of the file pls.
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.
@ncoden did not understand ... do you mean to give one line space at the end ?
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.
Yes. Always let one end-of-line (not an empty line) at the end of files. If you use an editor like Sublime Text or Atom, there is plugin to do that automatically
| $dropcaps-uppercase: true !default; | ||
|
|
||
| /// Dropcaps component | ||
| @mixin dropcaps($uppercase: $dropcaps-uppercase) { |
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.
/// Dropcaps component
/// @param { Boolean } $uppercase [$dropcaps-uppercase] If `true`, makes dropcaps uppercase.
@mixin dropcaps(
$uppercase: $dropcaps-uppercase
) {- Add comment for parameter
- Show one parameter per line
| /// Dropcaps component | ||
| @mixin dropcaps($uppercase: $dropcaps-uppercase) { | ||
| // Default margin (Arial/Helvetica) to let it properly aligned from the top on the | ||
| // first line beside it and have a tiny space between them through the right |
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.
// See: http://code.stephenmorley.org/html-and-css/creating-drop-caps/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.
@ncoden - If you are talking about the comment, i have used it from there only and my english isn't great. so would request you to give me the content for it and i will add ... Pleaseee ?
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 mean: just add the link below your comment.
| font-size: $-font-size; | ||
| line-height: $-line-height; | ||
|
|
||
| @if $dropcaps-uppercase { |
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.
@if $uppercase {(the parameter, not the global setting)
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.
ohk sure understood
| @mixin dropcaps($uppercase: $dropcaps-uppercase) { | ||
| // Default margin (Arial/Helvetica) to let it properly aligned from the top on the | ||
| // first line beside it and have a tiny space between them through the right | ||
| $-margin: 0.192em 0.0962em 0 0; |
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.
Hmm... Just seen it is not bidi-compliant (read in RTL and LTR).
We can use this instead:
$-margin-top: 0.192em;
$-margin-right: 0.0962em;
...
margin-top: $-margin-top;
margin-#{$global-right}: $-margin-right;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.
Hmmnnn.... you are right! .... oh i mean left ... oh sorry right 😈
|
hope @ncoden now its ohk .... ?? |
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.
@kball Waiting for your opinion for the questions above :)
|
|
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.
No empty line here
|
I requested you a lot of change, I know 😄. But I think I asked you everything that could be asked, so your next PR will be perfect ! It makes me think we should add a full guide for Sass and contribution in docs. We talked about it before with @kball. About the squatch, I would prefer to do it in merging, so the commit history in this PR will be preserved. It's the same for rebase. Before I recommended it but now I think it should be avoided. Again, this require more discussion about our workflow. I will add this question to #9576. |
|
Ohk no squash then, even i thought that no squash will preserve the commit history ! |
|
Ah, yes ! I forgot the docs ! |
Yup i thought that few more reviews, comments and commit, and this would have raced up to most commented pull request pretty quickly 😄 |
|
@kball : The development phase is over now ... |
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.
There's a lot of magic numbers, some complex formulas, and short, unintuitive variable names. Can we provide references for where the numbers and formulas are coming from? And potentially name the variables in a way that someone looking it this has a chance of understanding what they are?
| $-margin-right: 0.0962em; | ||
|
|
||
| // Font metrics (for Helvetica Neue) | ||
| $mt: (15.6 / 100); // Distance between Ascent and Top |
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.
Where do these magic numbers come from?
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.
Measure by yourself: https://codepen.io/ncoden/pen/mRJvEg ;)
I agree there is certainly a way to get the exact metrics of fonts. We should also declare them in Sass as a ~standard map in settings.
| $n: $lines; | ||
| $h: $n * $lh; | ||
|
|
||
| $-font-size: ($h - ($lh + $mt + $mb) + 1) / (1 - ($mt + $mb)); |
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.
Is there a reference for this formula?
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.
Or the one below it?
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.
cc / @ncoden
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.
Yes. I'm working on it.
| ) { | ||
| // Default margin right (Arial/Helvetica) for having a tiny space between them through the right, | ||
| // See: http://code.stephenmorley.org/html-and-css/creating-drop-caps | ||
| $-margin-right: 0.0962em; |
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.
If this is specific to Arial/Helvetica, what happens if someone is not using those?
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.
See the docs:
The initial letter has the best alignment with the paragraph lines for standard
serifandsans-seriffonts likeHelveticaorTimes New Roman. The render may differ for fonts with different metrics.
However, I have some idea to generate his value dynamically, to try to have the same space on the right (or left for LRT) of the letter than we have under it.
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 Have tested with many web fonts like Roboto, Open sans, Nato Sans, Ubuntu and oxygen and works well ! Infact i also tested with monospace and it works well there too. So there is no such font issue as per my best of knowledge and for sure this is not breaking #9596 development
Although i would also arque that these below private variables should be there as configuration variables so that user's can change it to there font needs !
- $-margin-right
- $mt
- $mbThoughts ?
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.
@IamManchanda Agreed. These values can "match" with the most common fonts, but these are still values for Helvetica Neue and there is no guarantee the render will be still good with other fonts.
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.
However, please warn when you are working on an issue
So are you doing this or shall i do this ?
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 am working on it.
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.
ohk will be waiting bro !
|
I didn't knew that foundation team members works so hard and like this for every single feature... Liked it very much, @IamManchanda @ncoden. |
|
@RamanCool We do not. I have a curse which force me to review and work on a pull request forever until it is perfectly perfect. |
|
Any update Nicolas ? {{ As always, you can take your time though }} |
|
I lost the whole day to learn how font metrics are provided in standard formats. And it appear that, even today, there is not a standard way to interpret these values. So we will have to provide a map of datas (ascender, descender, capital height) we measured ourself ! I said I am working on this. Do not worry 😄 . |
|
sorry to pinch you again but any updates bro ? 😄 |
|
I'm blocked on: https://codepen.io/ncoden/pen/mRJvEg, and waiting answers from people who know W3C better than me. The workaround will be to use our own font metrics (like we are currently) instead of standard OS/2 metrics. |
|
How about you opening a issue request for the same ⬆️ with reference to this pr and codepen, you might get answers or how about posting it on stack overflow? |
|
I will :) |
|
So did you got the answers @ncoden ? |
|
@ncoden any updates and what about including feature queries within it! @supports (initial-letter: 4) or (-webkit-initial-letter: 4) {
.dropcaps::first-letter {
-webkit-initial-letter: 4;
initial-letter: 4;
}=> https://hacks.mozilla.org/2016/08/using-feature-queries-in-css/ |
|
As nicolas has decided to take a break ... Secondly @ncoden |
Not sure if I understand correctly. We had typography in school and this is right what you see. Think about a printer using single letters. They all need to be aligned on the x line (the baseline). The space for the serif parts always is there. It is often just not used and is the leading between the lines (the white / grey factor and the space between them). The W3C spec says
Which is totally fine and expected. We can not redefine the bleedbox of a provided font which uses serif parts |
|
@DanielRuf The issue you replied to is resolved. I was understanding the W3C reference correctly but compared it with wrong measures and calculations --' |
Oh, was not aware of it as this PR is still open. |
|
Or are all other questions answered and this can be probably merged in one of the planned milestones? |
It is still open because I have to go back into the PR, clean the code, add documentation and resolve the last bugs. And this require me to understand what I did there (or remind it). |
|
Understood =) |
|
Given this was pending work from @ncoden and AFAIK he is no longer involved, I'm going to close this. |

Implementation for: #9592
Forwarded from: #9606 ( Due to branching issue )
This adds a new feature CSS Dropcaps,
Simply use a dropcaps class in any html tag and the first letter of the text collection will grow to a bigger size proportionate to
3 normal linesand is as similar to what is found in magazines and the modern web@ncoden @kball for review
Update: Here is the updated code from the below humongous code discussion b/w me & Nicolas
The Source Code behind all of this -
_dropcaps.scssGenerated CSS
And, the output (Helvetica font)
Mobile output
Desktop output