search

brackets-archive / bracketsIssues

Archive of issues in brackets.
0 stars 0 forks source link

[CLOSED] StatusBar.addIndicator() doesn't actually add the indicator #5210

Open core-ai-bot opened 3 years ago

core-ai-bot commented 3 years ago

Issue by njx Friday Oct 25, 2013 at 01:01 GMT Originally opened as https://github.com/adobe/brackets/issues/5682

Originally, when StatusBar.addIndicator() was implemented, it would actually add the indicator into the status bar. However, in d7bce2cee3241cf3152e15cbfbcf66476dac23cb, that line was removed, which is a breaking API change. It's not clear why it was removed.

(Edited 12/14/2013 by@lkcampbell to add repro steps)

OS Version: Mac OSX Mavericks

Brackets Version: Sprint 34

Repro Steps: Open Brackets source code, then open Dev Tools to Console and paste the following code snippet into the Console:

var StatusBar = require("widgets/StatusBar"),
    $foo = $("<div>FOO</div>");
StatusBar.addIndicator("foo", $foo, true, "", "foo tooltip");

Look at the right side of the editor status bar.

In the Dev Tools, Elements tab, look at the contents of status-indicators div tag.

Observed Result: The FOO div does not show up in the status bar and is not in the status-indicators div tag

Expected Result: The FOO div shows up in the status bar and is in the status-indicators div tag

core-ai-bot commented 3 years ago

Comment by njx Friday Oct 25, 2013 at 01:01 GMT

@larz0 - do you know why you removed this?

core-ai-bot commented 3 years ago

Comment by larz0 Friday Oct 25, 2013 at 02:09 GMT

@njx I think it was because it was inserting something else that's not supposed to be there. I can't remember what though. Could it be related to JSLint indicator?

core-ai-bot commented 3 years ago

Comment by larz0 Friday Oct 25, 2013 at 03:18 GMT

@njx I put it back in on this branch https://github.com/adobe/brackets/tree/larz/issue-5682 and there are no visible changes.

core-ai-bot commented 3 years ago

Comment by lkcampbell Wednesday Nov 06, 2013 at 14:33 GMT

Assigning this issue to myself to fix, I need resolved so I can address issue #5078.

core-ai-bot commented 3 years ago

Comment by lkcampbell Sunday Nov 10, 2013 at 03:27 GMT

@njx, question for you. What repro steps are we using to test it and make sure I fixed it? This is the second issue I have run into where I need to create test steps for a Brackets module and I have no idea how to do it.

Do you guys have some way to conduct a headless test on some specific piece of the Brackets API?

core-ai-bot commented 3 years ago

Comment by lkcampbell Saturday Dec 14, 2013 at 16:24 GMT

Edited the description to include repro steps.

core-ai-bot commented 3 years ago

Comment by lkcampbell Saturday Dec 14, 2013 at 17:10 GMT

@njx and@larz0 , this problem is caused by what@njx mentioned above: the $indicators.prepend($indicator); line was removed from StatusBar.addIndicator().

But I also discovered the reason why@larz0 pulled this code in the first place.

This is what the status bar indicators look like normally:

screen shot 2013-12-14 at 8 45 32 am

This is what the status bar indicators look like after inserting the prepend line again:

screen shot 2013-12-14 at 8 42 47 am

The problem is addIndicator() doesn't have a way to indicate relative position. The default behavior is to push indicators onto the right side. It is a little counterintuitive because children of the status-indicators div are drawn right to left. That is to say, the first child is drawn on the rightmost side and the last child in drawn on the leftmost side.

We must have some special code that sets the JSLint indicator position manually right now. Reactivating the default behavior of addIndicator() overrides it.

My question is, how do you want me to fix this problem? What is the expected behavior? If the JSLint indicator is to remain in the middle, I may have to add relative positioning to the addIndicator() method interface. Any other ideas?

core-ai-bot commented 3 years ago

Comment by lkcampbell Friday Dec 20, 2013 at 00:04 GMT

@njx or@redmunds, Randy was asking about the status of the "Reload Without User Extensions" feature today. This issue is one of the blocking issues for the feature. I am looking for feedback from you regarding the best way to address this problem. Please see my questions above.

core-ai-bot commented 3 years ago

Comment by njx Friday Dec 20, 2013 at 00:12 GMT

Sorry, forgot to follow up. A few thoughts:

core-ai-bot commented 3 years ago

Comment by lkcampbell Sunday Dec 22, 2013 at 15:35 GMT

@njx and@redmunds, fix committed.

core-ai-bot commented 3 years ago

Comment by redmunds Friday Dec 27, 2013 at 17:34 GMT

FBNC back to@njx

core-ai-bot commented 3 years ago

Comment by njx Thursday Jan 09, 2014 at 23:30 GMT

Haven't tried it myself but the code looks sensible. Closing.

core-ai-bot commented 3 years ago

Comment by peterflynn Friday Jan 10, 2014 at 01:54 GMT

@lkcampbell@redmunds Do you think this is clean enough to be an official API for use by extensions now? If so, would one of you mind adding it to the release notes as a new API?

core-ai-bot commented 3 years ago

Comment by redmunds Saturday Jan 11, 2014 at 18:57 GMT

@peterflynn Done.