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.

Sign up for GitHub

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

Jump to bottom

[CLOSED] Inline Editor Close Button #4998

Open
core-ai-bot opened this issue Aug 30, 2021 · 30 comments
Open

[CLOSED] Inline Editor Close Button #4998

core-ai-bot opened this issue Aug 30, 2021 · 30 comments

Comments

@core-ai-bot
Copy link
Member

Issue by VizuaaLOG
Monday Oct 07, 2013 at 19:15 GMT
Originally opened as adobe/brackets#5443

I have added a close button to the inline editor as was requested here adobe/brackets#5441. The unit test was successful. Please review the work and leave any feedback.

VizuaaLOG included the following code: https://github.com/adobe/brackets/pull/5443/commits
@core-ai-bot
Copy link
Member Author

Comment by MarcelGerber
Monday Oct 07, 2013 at 19:43 GMT

Works fine for me, unit tests were successful.

@core-ai-bot
Copy link
Member Author

Comment by larz0
Wednesday Oct 09, 2013 at 03:54 GMT

Thanks@VizuaaLOG, looks good.

@njx we should do the same for color editor as well right?

(Tagging@redmunds for the upcoming bezier editor.)

screen shot 2013-10-08 at 8 51 57 pm

@core-ai-bot
Copy link
Member Author

Comment by MarcelGerber
Wednesday Oct 09, 2013 at 06:12 GMT

If we do this in the color editor, we should also do this in WebDocs.
But I wonder if there is a way to ingegrate this in every inline editor, also if it is called ba an extension. That would maybe be a better solution.

@core-ai-bot
Copy link
Member Author

Comment by VizuaaLOG
Wednesday Oct 09, 2013 at 20:18 GMT

@larz0 Thanks :).

@SAPlayer I could look into if the inline editors have any classes in common and then add it in that way.

@core-ai-bot
Copy link
Member Author

Comment by njx
Wednesday Oct 09, 2013 at 20:27 GMT

@VizuaaLOG Take a look at InlineWidget (which is the base class for all the inline editors) and see if you can put it in there in a reasonable way.

@core-ai-bot
Copy link
Member Author

Comment by MarcelGerber
Wednesday Oct 09, 2013 at 20:56 GMT

@njx I tried this (b25fd99890ffcae5ecf3ed305a6ed6c78aaa70aa) - the result works for WebDocs, but not for the Inline Editor, because it is would be underneath the scrollbar (not visible). The problem is, we haven't got a default way/element for the InlineWidget. Every (default) extension has its own way to create an InlineWidget.

@core-ai-bot
Copy link
Member Author

Comment by VizuaaLOG
Wednesday Oct 09, 2013 at 21:29 GMT

@SAPlayer@njx I think it will work on all inline widgets. I moved the code into InlineWidgets.js and changed it slightly. I also edited the styles in brackets.less so that it should always be positioned in the same place.

@core-ai-bot
Copy link
Member Author

Comment by VizuaaLOG
Wednesday Oct 09, 2013 at 21:49 GMT

@RaymondLim Fixed the jslint errors.

@core-ai-bot
Copy link
Member Author

Comment by RaymondLim
Wednesday Oct 09, 2013 at 21:51 GMT

@VizuaaLOG Your close button works, but focus gets lost after clicking on it though. If you start typing after using the close button, nothing shows up in the editor.

@core-ai-bot
Copy link
Member Author

Comment by VizuaaLOG
Thursday Oct 10, 2013 at 17:37 GMT

I fixed the extra quote problem however the focus is still lost when clicking the close button. I thought about doing editor.focus() however the only way i can do this is by requiring the editor.js file in InlineWidget.js. If you have any other solutions please say. I am still currently looking for another way.

@core-ai-bot
Copy link
Member Author

Comment by larz0
Thursday Oct 10, 2013 at 17:59 GMT

@VizuaaLOG just letting you know we've decided to move the close button to the left so that it can be more consistent with other inline editors and quick doc. Thanks for you help!

4deb3b76-3096-11e3-82d1-3127cb78e806

@core-ai-bot
Copy link
Member Author

Comment by VizuaaLOG
Thursday Oct 10, 2013 at 18:08 GMT

@larz0 Ok, i fixed the focus problem. Also did you want me to commit it on the left or has it already been done?

@core-ai-bot
Copy link
Member Author

Comment by RaymondLim
Thursday Oct 10, 2013 at 18:19 GMT

@VizuaaLOG Larz is informing you to move the button to the left since you're working on it. So we're waiting for your commit.

@core-ai-bot
Copy link
Member Author

Comment by larz0
Thursday Oct 10, 2013 at 18:22 GMT

@VizuaaLOG yes please :)

@core-ai-bot
Copy link
Member Author

Comment by VizuaaLOG
Thursday Oct 10, 2013 at 18:27 GMT

Ok no problem will fix the problem with the entire widget being a close button then will commit the new changes

@core-ai-bot
Copy link
Member Author

Comment by VizuaaLOG
Thursday Oct 10, 2013 at 19:19 GMT

Just wondering if someone could help me :). I have changed the close button code to this

// create the close button
var closeBtn = "<a href='#' class='close' id='inline-widget'>&times;</a>";
closeBtn.click(function (e) {
    self.close();
    e.stopImmediatePropagation();
});
this.$htmlContent.append(closeBtn);

However i get the following error in dev tools Uncaught TypeError: Object <a href='#' class='close' id='inline-widget'>&times;</a> has no method 'click'. Am i missing something or have i misunderstood what@RaymondLim said?

@core-ai-bot
Copy link
Member Author

Comment by RaymondLim
Thursday Oct 10, 2013 at 19:35 GMT

You need to wrap your link element with $() and create an jQuery object and also prefix your variable with $ to indicate that it's a jQuery object.

@core-ai-bot
Copy link
Member Author

Comment by VizuaaLOG
Thursday Oct 10, 2013 at 19:44 GMT

Thanks it worked :) Just about to commit the changes

@core-ai-bot
Copy link
Member Author

Comment by MarcelGerber
Thursday Oct 10, 2013 at 19:52 GMT

I think you missunderstood him again (perhaps your method works, but it isn't nice). Maybe take a look at 0523bc60551de04392352b0769a1207f9a2f9860.

@core-ai-bot
Copy link
Member Author

Comment by VizuaaLOG
Thursday Oct 10, 2013 at 19:56 GMT

@RaymondLim ok will change it now also is@SAPlayer's method better because i think it looks cleaner so i will change it to that.

@core-ai-bot
Copy link
Member Author

Comment by RaymondLim
Thursday Oct 10, 2013 at 20:03 GMT

Yes,@SAPlayer's code is better as you're chaining to the existing code with the new close button first.

@core-ai-bot
Copy link
Member Author

Comment by VizuaaLOG
Thursday Oct 10, 2013 at 20:14 GMT

I think that is everything. I had to re commit the style because i undid the commits too far :(. But it all works again.

@core-ai-bot
Copy link
Member Author

Comment by VizuaaLOG
Thursday Oct 10, 2013 at 20:43 GMT

I have changed it to use the class and removed the ID and it all still works.

@core-ai-bot
Copy link
Member Author

Comment by VizuaaLOG
Thursday Oct 10, 2013 at 20:53 GMT

@RaymondLim Sorry i changed it, need to get used to that because i usually use single quotes.

@core-ai-bot
Copy link
Member Author

Comment by RaymondLim
Thursday Oct 10, 2013 at 20:55 GMT

Looks good now. Merging.

@core-ai-bot
Copy link
Member Author

Comment by VizuaaLOG
Thursday Oct 10, 2013 at 20:56 GMT

Ok thanks.

@core-ai-bot
Copy link
Member Author

Comment by larz0
Thursday Oct 10, 2013 at 21:01 GMT

🍻

@core-ai-bot
Copy link
Member Author

Comment by RaymondLim
Thursday Oct 10, 2013 at 21:19 GMT

Oops... Looks like this breaks one of the WebPlatformDocs extensions unit tests. File a new issue #5473 for the unit test failure.

@core-ai-bot
Copy link
Member Author

Comment by njx
Thursday Oct 10, 2013 at 21:19 GMT

I just checked this out and it looks fine everywhere except Quick Docs, where it seems to be causing the content in the right column to shift downward. Filed as #5472.

@core-ai-bot
Copy link
Member Author

Comment by VizuaaLOG
Thursday Oct 10, 2013 at 21:26 GMT

I just looked at the problem and it is the margin right to move the content away from the close button. However, i cant think of another way to move the content away from the button. Any ideas?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@core-ai-bot