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

Customizing embed aspect ratios #28106

Closed
ysds opened this issue Jan 23, 2019 · 6 comments · Fixed by #28678
Closed

Customizing embed aspect ratios #28106

ysds opened this issue Jan 23, 2019 · 6 comments · Fixed by #28678
Labels

Comments

@ysds
Copy link
Member

ysds commented Jan 23, 2019

The way to customize the embed aspect ratios is some confusing.

(1) The following SCSS occurs compile ERROR:

$embed-responsive-aspect-ratios: (2 1);

@import "../node_modules/bootstrap/scss/bootstrap";

Error message:

List index is 2 but list is only 1 item long for `nth'

Demo: https://www.sassmeister.com/gist/d8f4ece6f886602fc110c594a33357f5

(2) The following SCSS is OK:

$embed-responsive-aspect-ratios: (2 1), ;

@import "../node_modules/bootstrap/scss/bootstrap";

(3) The following SCSS is OK:

$embed-responsive-aspect-ratios: (2 1), (3 1);

@import "../node_modules/bootstrap/scss/bootstrap";

SASS seems to recognize $list: (2 1) as a list included the two values instead of one set of two values.

Demo: https://codepen.io/anon/pen/GzgrwL?editors=1111 - See console view

@pcodedviral
Copy link

hello @ysds

you can not add single value in list variable

plz use multiple list values

@ysds
Copy link
Member Author

ysds commented Jan 23, 2019

Yeah, I understand the usage of the Sass list. This is not a question but a issue report about the documents or SCSS.

@MartijnCuppens
Copy link
Member

It's just not easy to define a list with one list as value in sass. I would go for $list: append((),(2 1)), but yeah, that's just something you got to know. Maybe we should switch to sass maps instead of lists? Something like this:

$embed-responsive-aspect-ratios: ("2by1": (2 1))

This would be a breaking change

What do you think about that, @ysds?

@ysds
Copy link
Member Author

ysds commented Jan 29, 2019

I prefer to use sass map for v5, because it can create semantic class names and can use a number containing a decimal separator. e.g.:

$embed-responsive-aspect-ratios: (
  "square": 1 1,
  "golden": 1.618 1,
  "silver": 2.414 1,
)

@MartijnCuppens MartijnCuppens added v5 and removed v4 labels Jan 31, 2019
@MartijnCuppens
Copy link
Member

Hi @martinbean, this is related to your comment on #25894. It's not easy to define a sass list with one item. The most straightforward method is $embed-responsive-aspect-ratios: append((), (4 3)) I guess.
We'll probably change this in v5 and will use sass maps like ysds describes above.

But I think we'll also need to add a notice about this in our docs for v4 since it seems a lot of people are having troubles with this.

@martinbean
Copy link
Contributor

Wow. That is some clunky syntax but thanks for pointing me in the right direction, @MartijnCuppens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants