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

Optionally preserve line height #30

Merged

Conversation

trumbitta
Copy link
Contributor

As described on #29, this adds an attribute and configuration to optionally set the line-height to the original font-size value just after the resizing has happened.

This helps when you have several elements of different lengths aligned and want to keep the alignment once they get resized.

Think a grid of big titles getting resized, with small subtitles not getting resized.
Without this, the subtitles would end up misaligned to each other on the grid.

This comes in handy when you have several aligned elements of different lengths, that will be resized to different font-sizes.
Setting back the line-height to the original font-size helps keeping such elements aligned.
@patrickmarabeas
Copy link
Owner

Awesome! Thanks for the PR - I'll have a look ASAP.

Any chance you could add an example of this functionality to the demo page included so I can quickly get up to speed and confirm I'm on the same page as you?

Thanks!

@trumbitta
Copy link
Contributor Author

Here you go :)

I'd also like to add some tests, but I couldn't make the suite work :-/

@patrickmarabeas
Copy link
Owner

Thanks!

Versions may well be incompatible with whatever version of Angular is in there they are so old. Fair warning - I wouldn't think any of the tests would currently pass.

@trumbitta
Copy link
Contributor Author

Ok, this was my last commit until your feedback :)

Using the original font-size to set the line-height was working for me, just because my line-height was actually 1.

It just occurred to me that using the actual original line-height is more correct 👍

@trumbitta
Copy link
Contributor Author

Hey :) any news?

@patrickmarabeas patrickmarabeas merged commit b73d429 into patrickmarabeas:master Jan 17, 2016
@patrickmarabeas
Copy link
Owner

Thanks for the PR! I've finally merged in ; )

As a note, I'll be simplifying the functionality to be standard rather than optional (along with the display property and fittext-nl attribute). This will be pushed out in v4.

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