Skip to content

Utility API improvement: use $key/$value instead of nth#28828

Merged
MartijnCuppens merged 16 commits into
twbs:masterfrom
ashfahan:master
May 30, 2019
Merged

Utility API improvement: use $key/$value instead of nth#28828
MartijnCuppens merged 16 commits into
twbs:masterfrom
ashfahan:master

Conversation

@ashfahan

Copy link
Copy Markdown
Contributor

instead of zipping values together convert list or strings to map and then use them

@ashfahan ashfahan requested a review from a team as a code owner May 26, 2019 23:37
@ashfahan

Copy link
Copy Markdown
Contributor Author

@mdo @ysds @MartijnCuppens any reviews over pr . thanks 😅

@MartijnCuppens

Copy link
Copy Markdown
Member

@ashanbrown, no need to mention us here, patience is your best friend.

I don't think the new function has a lot of added value.

But I like what you did with the $key/$value thing, could you just tackle that in this PR?

@ashfahan

Copy link
Copy Markdown
Contributor Author

sorry about that @MartijnCuppens . i was just curious .

about this pr yeah you are right it doesn't really add or remove much from workflow . however its main reason is to increase the readability and better porting for various other things that needs to be converted to maps and around

Thanks for your time.

@MartijnCuppens

Copy link
Copy Markdown
Member

The problem is this PR adds about 50 lines to our codebase that 'll need to be maintained by us and doesn't provide that much of a benefit (it's only used in one place).

Let's just keep the $key/$value thing you did, that's a small improvement which does make the code easier to understand.

@ashfahan ashfahan changed the title Add an actual function to convert lists or strings to maps Use $key/$value instead of nth May 29, 2019
@ashfahan

Copy link
Copy Markdown
Contributor Author

you got a point 😅 .

Pr updated

Comment thread scss/mixins/_utilities.scss Outdated
Comment thread scss/mixins/_utilities.scss Outdated
Co-Authored-By: Martijn Cuppens <martijn.cuppens@gmail.com>

@MartijnCuppens MartijnCuppens left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@MartijnCuppens MartijnCuppens changed the title Use $key/$value instead of nth Utility API improvement: use $key/$value instead of nth May 30, 2019
@MartijnCuppens MartijnCuppens merged commit 41fcd13 into twbs:master May 30, 2019
@ashfahan

Copy link
Copy Markdown
Contributor Author

Thanks!

It's my pleasure.

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 this pull request may close these issues.

2 participants