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

New emmet: css property width expands to widows?? #28933

Closed
jens1o opened this issue Jun 17, 2017 · 14 comments
Closed

New emmet: css property width expands to widows?? #28933

jens1o opened this issue Jun 17, 2017 · 14 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug emmet Emmet related issues verified Verification succeeded
Milestone

Comments

@jens1o
Copy link
Contributor

jens1o commented Jun 17, 2017

  • VSCode Version: Code - Insiders 1.14.0-insider (73015a5, 2017-06-16T05:13:31.107Z)
  • OS Version: Windows_NT ia32 10.0.15063
  • Extensions:
Extension Author (truncated) Version
path-intellisense chr 1.4.2
gitlens eam 4.1.2
LogFileHighlighter emi 1.1.1
vscode-reveal evi 0.0.9
php-debug fel 1.10.0
php-intellisense fel 1.4.1
auto-close-tag for 0.4.2
gc-excelviewer Gra 1.1.15
GitHubIssues Hoo 0.1.2
composer ika 0.5.0
smarty imp 0.2.0
json-to-ts Mar 1.4.3
vscode-apache mrm 1.1.1
cpptools ms- 0.11.4
php-docblocker nei 1.2.0
vscode-versionlens pfl 0.19.0
vscode-statusbar-json-path ric 1.0.7
vscode-icons rob 7.9.0
sharecode Rol 0.4.1
code-spell-checker str 1.2.0
vscode-todo-highlight way 0.5.5
highlight-trailing-white-spaces yba 0.0.2

(1 theme extensions excluded)


image
have fun.

// cc @ramya-rao-a

@vscodebot vscodebot bot added the insiders label Jun 17, 2017
@ramya-rao-a
Copy link
Contributor

I am having a deja vu :)

w gives width
wi and anything further gives widows

This is coming from the way snippets are defined in https://github.com/emmetio/snippets/blob/master/css.json

@sergeche thoughts?

cc @sergeche

@ramya-rao-a ramya-rao-a added emmet Emmet related issues upstream Issue identified as 'upstream' component related (exists outside of VS Code) labels Jun 17, 2017
@ramya-rao-a ramya-rao-a self-assigned this Jun 17, 2017
@druellan
Copy link

I'm starting to get similar results with other properties:

2017-06-17 13_08_43- _footer scss wp-content visual studio code - insiders

the default hint for font-family is font: fantasy; which is a very loose association and ruins the hinting system

@jens1o
Copy link
Contributor Author

jens1o commented Jun 17, 2017

wi and anything further gives widows

but then I would expect that the label updates too...

@sergeche
Copy link

Not sure what expected behaviour should be if you type full property name and try to expand it as Emmet abbreviation. CSS itself has too much clashing properties/abbreviations. The goal of shortcuts is to provide shorthands for most commonly used properties. Users should learn these shortcuts by typing first letters and seeing autocomplete popup with available options. If the full property names will be considered in abbreviation resolving, they will collide with shorthands, especially ones with embedded shorthand values, like m-a for margin: auto.

So the ultimate goal is to provide matched snippets and expanded abbreviation with previews

@ramya-rao-a
Copy link
Contributor

ramya-rao-a commented Jun 21, 2017

Another example:

screen shot 2017-06-21 at 9 04 24 am

User expectation would be to match "padding"

@sergeche In this case the only match between "padd" and the suggested shorthand "pgba" is the first letter "p". Aren't we being too eager while matching here?

@ramya-rao-a ramya-rao-a added this to the July 2017 milestone Jun 22, 2017
@vvs
Copy link

vvs commented Jun 23, 2017

Indeed, this eager matching to the Emmet shorthands severely breaks (this) user expectations.

When I have almost complete property name, like "borde", "paddin", "margi", etc, I would assume that VS Code does the right thing and expands these widely-used properties. Instead, I get completely random and unrelated (at least that's how it looks) suggestions from Emmet.

My expectation would be that Emmet's suggestions should not always be the number 1 in the suggestion list, and sometimes they should be ranked lower as compared to the almost fully-matched widely used properties.

Suggesting "pgba" shorthand as a top choice for "paddin" is just wrong (at least, to me).

@ramya-rao-a
Copy link
Contributor

ramya-rao-a commented Jun 24, 2017

One solution I have in mind here is for the css completion provider (from the css extension that is built in) to call the emmet completion provider (See #29114 (comment)) instead of the emmet extension.

As the css extension will also have in hand the other suggestions, it can add some logic to filter/sort appropriately.

I'll look into that next week.

Until then, I am pushing the below changes

  • The label in the completion item for emmet will now show the expanded abbreviation and not the abbreviation itself. With this, it will be more clear as to what exactly is the user choosing.
  • For sorting, the expanded abbreviation will be used and not the abbreviation itself.

The changes will be out in Monday's Insiders build.

Please give it a try then and let me know how the improvements work out for you.
If things are reasonable, then I'll push the work on the css completion provider to next iteration.

Again, thanks for all the feedback!

@jens1o
Copy link
Contributor Author

jens1o commented Jun 25, 2017

temporary fixed with microsoft/vscode-emmet-helper@2bbf018

@ramya-rao-a
Copy link
Contributor

I have logged #29113 to track the work of moving emmet completion provider to css language service.

Closing this issue for the June milestone as the improvements listed in #28933 (comment) should help most of the cases.

@ramya-rao-a ramya-rao-a added bug Issue identified by VS Code Team member as probable bug and removed upstream Issue identified as 'upstream' component related (exists outside of VS Code) labels Jun 26, 2017
@ramya-rao-a
Copy link
Contributor

Note to verifier:

  • In css like files (css/scss/less), ensure that the suggestion label reflects the actual completion rather than just the abbreviation
  • Ensure the suggestion from emmet is sorted among the css suggestions

@vvs
Copy link

vvs commented Jun 26, 2017

Hi @ramya-rao-a , many thanks for the improvements, they significantly improve the user experience w.r.t editing CSS/SCSS files!

The sorting of the suggestions is much, much better now.

There is one issue though (actually two), regarding snippets.
Say, I have a snippet called bidi (for bi-directional SCSS mix-ins).

== Issue 1 ==

I type 'b' and see the following:

vc-emmet-1

The help for my snippet bidi is wrong.

== Issue 2 ==

When I type the full name of the snippet, bidi in this case, it is still listed below the Emmet suggestion (which seems completely unrelated to the name of my snippet):

vc-emmet-2

Should I file separate issues for that?

@ramya-rao-a
Copy link
Contributor

ramya-rao-a commented Jun 26, 2017

@vvs wow, the first issue is seriously messed up, thanks for reporting!
This is a separate issue indeed. I have logged #29468

@ekfuhrmann
Copy link

I'm still running into issues with emmet and specifically the width shorthand.

white-space and widows both get priority over width which arguably is one of the most commonly used properties. Is there any way we can reorder the priority and shift some of these lesser-used properties further down the list?

Gif below to show what I'm seeing...

emmet-width

@ramya-rao-a
Copy link
Contributor

@ekfuhrmann You can set a custom emmet snippet for wid

{   
    "css": {
        "snippets": {
            "wid": "width:${1};"
        }
    }
}

See Using Custom Snippets in Emmet

Or you can take advantage of emmet and instead of typing wid you could say type w10 which would expand to width: 10px (you can use any number instead of 10)

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug emmet Emmet related issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

7 participants