Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

CSS code hints in 'style' attribute value context #13270

Merged
merged 2 commits into from
Apr 13, 2017

Conversation

swmitra
Copy link
Collaborator

@swmitra swmitra commented Apr 5, 2017

This PR enables Brackets to provide CSS code hints property-value code hints while editing 'style' attribute value in 'html' mode. The implementation contains minor tweaks to the CSSUtils module to recognize this as 'css' context as CodeMirror doesn't understand the 'style' attribute value as 'css' context.

Adding @ingorichter @petetnt @zaggino and @ficristo for review, but others can also join 馃憤

@ficristo
Copy link
Collaborator

ficristo commented Apr 6, 2017

Sorry but I don't have enough interest in this feature to review it.
Maybe @humphd is interested?

@ficristo ficristo removed their request for review April 6, 2017 18:43
@swmitra
Copy link
Collaborator Author

swmitra commented Apr 7, 2017

@humphd Can you please have a look at this PR?

// and in attribute value state of a tag with attribute name style
if (ctx.token.state.htmlState && (!ctx.token.state.localMode || ctx.token.state.localMode.name !== "css")) {

// tagInfo is required to aquire the style attr value
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: indent level

Copy link
Collaborator

Choose a reason for hiding this comment

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

Indent

@ingorichter
Copy link
Contributor

@swmitra it would be beneficial to have some unit tests for the new functionality. This is complex and might break accidentally.

@swmitra
Copy link
Collaborator Author

swmitra commented Apr 9, 2017

Sure @ingorichter. I will add some functional tests in this PR and ping you.

Copy link
Collaborator

@petetnt petetnt left a comment

Choose a reason for hiding this comment

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

Change set LGTM with a small nit.

However it doesn't seem to handle multiline styles yet:

<div style="background-color: red; // works here
color: white; // doesn't work here>Foo</div>

It would be also great if it worked on JSX context (<div style={{backgroundColor: red}}>Hey`) but that's more a feature request :)

// and in attribute value state of a tag with attribute name style
if (ctx.token.state.htmlState && (!ctx.token.state.localMode || ctx.token.state.localMode.name !== "css")) {

// tagInfo is required to aquire the style attr value
Copy link
Collaborator

Choose a reason for hiding this comment

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

Indent

@swmitra
Copy link
Collaborator Author

swmitra commented Apr 12, 2017

@ingorichter I have added some unit tests in my last commit.
@petetnt Fixed the multiline attribute value issue and corrected indentation.
Can you guys please review it once more? 鈽猴笍

Copy link
Contributor

@zaggino zaggino left a comment

Choose a reason for hiding this comment

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

  • checked the code, seems good
  • checked the unit tests, present & passing
  • tested the feature manually, working fine

@zaggino zaggino merged commit 387731b into master Apr 13, 2017
@zaggino zaggino deleted the swmitra/StyleAttributeHints branch April 13, 2017 04:44
@ingorichter
Copy link
Contributor

I'm too late :-/

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

Successfully merging this pull request may close these issues.

None yet

5 participants