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

Wrap with Abbreviation return error #1

Closed
ArkadiuszMichalski opened this issue Feb 25, 2016 · 18 comments
Closed

Wrap with Abbreviation return error #1

ArkadiuszMichalski opened this issue Feb 25, 2016 · 18 comments

Comments

@ArkadiuszMichalski
Copy link
Contributor

On Win7 64bit, IE11, NPP 6.9 + last jN and your Emment I see that when run "Wrap with Abbreviation" I get error... I try figured out what is going on and catch this line in Dialog.js:
20 styles.styleSheet.cssText = this.cfg.css || '\nbody {overflow: auto;}\n';
so change them to:
20 styles.textContent = this.cfg.css || '\nbody {overflow: auto;}\n';
and works.

Btw., is there any solution to remember last position and sizes of this window? Every time when I run this command I get default window (somewhere in the middle in my screen)?

@eight04
Copy link
Owner

eight04 commented Feb 25, 2016

I can't get any error, but the style isn't applied. You may have to report it to jN.

to remember last position and size of the window

I will try this later.

@ArkadiuszMichalski
Copy link
Contributor Author

Sure, but style element doesn't have styleSheet property (per HTML/CSS), so when trying use styles.styleSheet.cssText generate error, so better use styles.cssText or styles.textContent. It rather depends on the version of IE in the system. Oh right, this fits more to jN author.

Hmm I have few questions:

  1. What version of Emmet you used?
  2. Option 'Update Image Size' works in NPP? I try with some absolute and relative path but without any result. Even when I put image file in the same folder where my html file and use <img src="test.gif" alt=""> nothing happend. The same is for "Encode\Decode data:URL image".
  3. Will be any config (via json) to reoragnize all position in Emmet menu? I don't want change orginal source because I will have to this for each update. Orginal names can state, but posibility mark them with some position index (0, 1, 2, 3....) or just read exact order from json config will be nice. Possibility to creat own submenu or maybe some separator would also be welcome.

@eight04
Copy link
Owner

eight04 commented Feb 25, 2016

  1. What version of Emmet you used?

Latest commit with branch master. Currently it is emmetio/emmet@3a946a2

  1. Option 'Update Image Size' works in NPP? I try with some absolute and relative path but without any result. Even when I put image file in the same folder where my html file and use nothing happend.

I will look into this.

  1. Will be any config (via json) to reoragnize all position in Emmet menu? I don't want change orginal source because I will have to this for each update. Orginal names can state, but posibility mark them with some position index (0, 1, 2, 3....) or just read exact order from json config will be nice. Possibility to creat own submenu or maybe some separator would also be welcome.

It uses emmet's builtin menu. It looks like:

[
    {
        type: "...", // submenu|item
        label: "...", // the label of the item, use .name for fallback
        name: "...", // action name,
        items: [...] // children items of submenu
    },
    ...
]

Maybe I can make it work like keymap.

@eight04
Copy link
Owner

eight04 commented Feb 25, 2016

Update Image Size

It looks like a bug in emmet. It doesn't provide size value to file.read.

https://github.com/emmetio/emmet/blob/master/lib/action/updateImageSize.js#L95
https://github.com/emmetio/emmet/blob/master/lib/interfaces/IEmmetFile.js#L26

@eight04
Copy link
Owner

eight04 commented Feb 25, 2016

Fixed in v0.2.0

@eight04 eight04 closed this as completed Feb 25, 2016
@ArkadiuszMichalski
Copy link
Contributor Author

Thx for fast fix, now I can jump from old Zen Coding to Emmet and make more test.

Btw., "Encode\Decode data:URL image" now works too, but I'm not sure what decode should do, because now in NPP I don't see any result, in Emmet example display dialog box for new path.
http://docs.emmet.io/actions/base64/
and description:
"With Emmet, you can convert image to data:URL right in your editor, as well as convert it back to external file." << if intention is backing to oryginal file then before encode this path should be somehow remember. Maybe display this dialog box with initial orginal (previous value) but we still have possibility pass other value?

Ah and one more thing, can you add to emmet.menu.json some marker for separator? I see that jN have that possibility
https://github.com/sieukrem/jn-npp-plugin/wiki/Working-with-menu#creating-separator
Maybe check some additional boolean property (like separator: true) and if exist then append separator after this option? I really prefers more separator than nested menu << need less clicking.
Edit: omg I see that separator exist, thx:)

@eight04
Copy link
Owner

eight04 commented Feb 25, 2016

Btw., "Encode\Decode data:URL image" now works too, but I'm not sure what decode should do, because now in NPP I don't see any result, in Emmet example display dialog box for new path.
http://docs.emmet.io/actions/base64/
and description:
"With Emmet, you can convert image to data:URL right in your editor, as well as convert it back to external file." << if intention is backing to oryginal file then before encode this path should be somehow remember. Maybe display this dialog box with initial orginal (previous value) but we still have possibility pass other value?

Emmet will use the path to save the image file to disk and I don't think it would remember the original path. Also, it is using a synchronously prompt, I don't know how to implement it in jN:
https://github.com/emmetio/emmet/blob/master/lib/action/base64.js#L80

Currently I just always send empty string:
https://github.com/eight04/jn-npp-emmet/blob/master/src/emmet.js#L529

@ArkadiuszMichalski
Copy link
Contributor Author

Hmm, so is any difference beetwen this "synchronously prompt" and prompt using in:
https://github.com/emmetio/emmet/blob/master/lib/action/wrapWithAbbreviation.js
Dialog.js doesn't fit in this scenerio?

@eight04
Copy link
Owner

eight04 commented Feb 25, 2016

In wrapWithAbbreviationAction, It will prompt only if the plugin doesn't send abbreviation to it (i.e. if abbr is empty), which means it will never use prompt api since we can get the user input before running the action.
https://github.com/emmetio/emmet/blob/master/lib/action/wrapWithAbbreviation.js#L29

But we can't do this with "Encode\Decode data:URL image", you can see that the prompt is always called when running decodeFromBase64.
https://github.com/emmetio/emmet/blob/master/lib/action/base64.js#L80

Dialog.js is a JavaScript module wrapping System.createDialog, which doesn't block script running.

IMHO, it would be better if emmet can change its interface. Using callback is much more convenient in JavaScript.

@eight04
Copy link
Owner

eight04 commented Feb 26, 2016

It looks like a bug in emmet. It doesn't provide size value to file.read.

https://github.com/emmetio/emmet/blob/master/lib/action/updateImageSize.js#L95
https://github.com/emmetio/emmet/blob/master/lib/interfaces/IEmmetFile.js#L26

I have to correct that the size param is optional, so there is nothing wrong with the API.

@eight04
Copy link
Owner

eight04 commented Feb 26, 2016

emmetio/emmet#431

@ArkadiuszMichalski
Copy link
Contributor Author

Oh thx for nice explanation, now understand this trick with Dialog.js when we don't have builtin prompt command. Relative size param I just started to write about it but before sending message I get new version with all changes, heh, so once again thx to you for all great job.

@ArkadiuszMichalski
Copy link
Contributor Author

Hmm now see that still is something wrong. Steps to reproduce:

  1. Open some html file.
  2. Run "Wrap with Abbreviation" and put some abbr or just click ok.
  3. Once again run "Wrap with Abbreviation".
    Last steps always produce error: null is empty or it's not a object; line: 131 pos: 7. Probabbly in entry.value = dialog.options.value || "";
    @eight04 Can you confirm or something is wrong on my side?

@eight04
Copy link
Owner

eight04 commented Feb 26, 2016

I can't reproduce it. From the message it looks like entry is null so it can't get entry.value. Can you confirm this?

@ArkadiuszMichalski
Copy link
Contributor Author

Problem is this 130 line:
var entry = dialog.handle.document.getElementById("entry");
When I first open dialog then entry is HTMLInputElement and is always this element when I close dialog box with "X" or Cancel button. But If I press OK (with or without any value) then when next time open dialog this line throw error << next line, because entry is not HTMLInputElement, probably its null. After this I always get this error when next time try open dialog and only run NPP once again repair situation.

@ArkadiuszMichalski
Copy link
Contributor Author

Hmm I test this on virtual machine (Win7 and IE8) and all works fine, so once again it's browser problem. You should test this minimum with IE11:)

Edit: ok, find what is going on... you use not standard property for event (e.returnValue = false;), should be e.preventDefault(). For old IE you can put some detect, like:
e.preventDefault ? e.preventDefault() : (e.returnValue = false);
or just return false; after d.close();
http://stackoverflow.com/questions/1000597/event-preventdefault-function-not-working-in-ie

@eight04
Copy link
Owner

eight04 commented Feb 27, 2016

I use IE11 with windows 7. Currently git hangs so I can't commit the edits 😟

Could you try replacing this line https://github.com/eight04/jn-npp-emmet/blob/master/src/emmet.js#L113 with:

e.preventDefault();

@ArkadiuszMichalski
Copy link
Contributor Author

But e.preventDefault(); not work in older IE, so my advise is just return false (works in all version) or use some detection (maybe once in the top, to not repeat this action xxx time).

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

No branches or pull requests

2 participants