Remove "x" buttons from dialogs

[le717]

This fixes #165. I refrained from clearing any white space from blank/end of lines.

@redmunds Do you need to review this, and do I need to fill out a CLA or anything first?

[redmunds]

@le717 Thanks. Yes, you need to agree to the Brackets CLA. Let me know when you have done this.

Yes, I will review your code, post any comments I have, and merge it when everything looks good.

[le717]

@redmunds Filled out the CLA. I was unable to find a CONTRIBUTING.md in the project, but I figured I'd ask anyway. Better safe than sorry. :)

[redmunds]

Done with my initial review.

In the Browse Web Fonts... dialog, the Done button acts as an OK button to insert the font into the page, and then dismiss dialog. They way your code works is it just dismisses the dialog. So, in that dialog, you'll need to add a Cancel button to replace the "x" button.

I think the other 2 dialogs are ok, but please double-check that I'm not forgetting any functionality.

[le717]

Added the "Cancel" text locale from the Brackets translations, and the other dialog's functionality are fine (howto only give directions, and include has you copy the text to your clipboard and insert it). As for the new button, where should I put it? The Edge Web Fonts ToS link on the left prohibits adding it on the traditional left side of the dialog.

[le717]

I've pushed the code that adds the cancel button, so if you want to review again you may. I am trying to test the button's actions but I cannot seem to clear my selected font from the include dialog...

[redmunds]

The group of OK (or Done) and Cancel buttons should be floated to the right as you have it in the screenshot above. But the Cancel button should on the right for Windows and the left for Mac. I am pretty sure that the modal dialog code fixes that for you across operating systems, but it looks like that screen shot was taken on Windows, so you need to swap the order of the buttons (note: when floating right, the elements are displayed in reverse order).

[le717]

Yes sir, I am using Windows 8.1 x64.

OK, so I swap the button location in the HTML. Since the modal dialog code changes the location of buttons on different platforms, then I do not need to attach float: right to it, correct? I want to make sure I am doing it correctly before I make another commit.

[le717]

Changes have been pushed, proceed when able. :)

[redmunds]

This code is overriding the OK button behavior -- it cancels instead of inserting font. I think this needs to be:

            dlg.getElement().find('.dialog-button[data-button-id="cancel"]').on("click", dlg.close.bind(dlg));

[le717]

Now that you point that out, it does appear it should be ('.dialog-button[data-button-id="cancel"]'), because the click command is to close the dialog, considering lines 453 and 454 handle what happens if the OK button is pressed.

[redmunds]

Sorry for just now realizing, but the Cancel button is already handled by default, so this is not necessary at all. This line of code should be removed.

[redmunds]

Sorry for the piecemeal code review. These should be the last changes.

[le717]

So it looks like I want all the way around my elbow to reach my ear, eh? Oh well, it happens. The review was fine. This is a new experience for me, contributing to another project and have others review my changes. It worked out. :)

Changes pushed.

[redmunds]

Thanks for updating the strings files.

One last change: Need to increment the version number in package.json here to 0.1.4 to the Extension Manager sees the update.

[le717]

Version increased, and you're welcome. :)

[redmunds]

Looks good. Thanks! Merging.

@iwehrman Here's another update to post to the registry. Thanks.

[iwehrman]

Done.