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

Add tag class attribute #166

Merged
merged 7 commits into from Sep 21, 2017

Conversation

Projects
None yet
3 participants
@abaracedo
Contributor

abaracedo commented Sep 13, 2017

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

This PR adds a specific class to the class attribute and let people style it as they wish. The resultant HTML code generated is this:

<span class="syntax--meta syntax--attribute-with-value syntax--clsss syntax--html">
  <span class="syntax--entity syntax--other syntax--attribute syntax--name syntax--class syntax--html">class</span>
  <span class="syntax--punctuation syntax--separator syntax--key-value syntax--html">=</span>
  <span class="syntax--string syntax--quoted syntax--double syntax--html">
    <span class="syntax--punctuation syntax--definition syntax--string syntax--begin syntax--html">"</span>
    <span class="syntax--meta syntax--toc-list syntax--class syntax--html">btn</span>
    <span class="syntax--punctuation syntax--definition syntax--string syntax--end syntax--html">"</span>
  </span>
</span>

I also updated the spec recognizes attributes to use a different attribute than class.

Alternate Designs

I am not totally convinced of the code because it is a copy & paste of the tag-id-attribute and replacing id to class in the contentName key. It would be great to know if there is any possibility to refactor in some way and only specify the contentName in each attribute (id and class.

Benefits

The option to style the class attribute.

Possible Drawbacks

None.

Applicable Issues

#48 but this PR does not add the style it, that has to be done on the syntax theme repositories.

@50Wliu 50Wliu added the needs-review label Sep 18, 2017

@abaracedo

This comment has been minimized.

Show comment
Hide comment
@abaracedo

abaracedo Sep 18, 2017

Contributor

Rebased and fixed tests.

Contributor

abaracedo commented Sep 18, 2017

Rebased and fixed tests.

Show outdated Hide outdated grammars/html.cson Outdated
Show outdated Hide outdated grammars/html.cson Outdated
Show outdated Hide outdated grammars/html.cson Outdated

@50Wliu 50Wliu added under-review and removed needs-review labels Sep 21, 2017

@50Wliu

This comment has been minimized.

Show comment
Hide comment
@50Wliu

50Wliu Sep 21, 2017

Member

This looks good - I'll merge once you fix the specs.

Member

50Wliu commented Sep 21, 2017

This looks good - I'll merge once you fix the specs.

@abaracedo

This comment has been minimized.

Show comment
Hide comment
@abaracedo

abaracedo Sep 21, 2017

Contributor

specs fixed :)

Contributor

abaracedo commented Sep 21, 2017

specs fixed :)

@50Wliu

50Wliu approved these changes Sep 21, 2017

@50Wliu 50Wliu merged commit 7179208 into atom:master Sep 21, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment