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

Fix modifier portion of BEM #417

Closed
wants to merge 1 commit into from
Closed

Fix modifier portion of BEM #417

wants to merge 1 commit into from

Conversation

conatus
Copy link

@conatus conatus commented Apr 9, 2015

The current implementation does not understand the "modifier" portion of the BEM syntax.

I copied out the BEM regex as in the code to test it http://regexr.com/3apmp

As you can see adding a simple modifier in the standard BEM way doesn't work.

The implementation I have done here fixes it as the follow regex shows http://regexr.com/3apms

Thanks very much.

@sds sds added the question label Apr 9, 2015
@sds
Copy link
Owner

sds commented Apr 9, 2015

Hey @conatus,

I might be mistaken, but I believe the hyphenated_BEM syntax is what you are looking for? Please see the discussion in #335.

@sds sds closed this Apr 9, 2015
@conatus
Copy link
Author

conatus commented Apr 9, 2015

Thanks for this - at least I made a PR to fix this "problem" amongst the other tickets!

Perhaps you could make this clearer in the documentation or alternatively rename BEM to "classic/strict/orthodox BEM" or something? Every tutorial I have seen, including some very recent ones uses the format that diverts slightly from the original formulation. The fact I wasn't the first to have this problem illustrates something is afoot.

@sds
Copy link
Owner

sds commented Apr 12, 2015

That's a fair point, @conatus. You're definitely not the first to be tripped up by this and certainly won't be the last unless we do something.

I like your suggestion of renaming the convention. I've chosen strict_BEM and made the change in 4d253c8. Thanks!

@conatus
Copy link
Author

conatus commented Apr 13, 2015

Thanks very much, this is cool.

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.

None yet

2 participants