Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Fix for issue #104 on the mac. #1969

Merged
merged 2 commits into from
Nov 2, 2012
Merged

Conversation

jeffslofish
Copy link
Contributor

Close file icon is not displayed until mouse is moved.

Fix for issue #104 on the mac.

@ghost ghost assigned redmunds Oct 28, 2012
@redmunds
Copy link
Contributor

Jeffrey,

Thanks for taking this one. I'm the one that filed he original bug, so obviously it bugs me :)

I think hard-coding the second param works for you because you only tested closing the file with the x icon. Try this:

  1. Open 2 files in working set
  2. Make second file dirty
  3. Select first file
  4. Close first file using File > Close

Results: Remaining file should have dirty flag, but it has x icon.

It has the x icon because the third param is hard-coded to true. After fixing that, the dirty flag still is not set because the second param is hard-coded to false. Both need to be determined. Here's what I did, but give it some more testing to see if there are other cases:

var $nextListItem = $listItem.next();
if ($nextListItem && $nextListItem.length > 0) {
    var canClose = ($nextListItem.find(".can-close").length === 1);
    var isDirty = isOpenAndDirty($nextListItem.data(_FILE_KEY));
    _updateFileStatusIcon($nextListItem, isDirty, canClose);
}

Note that $listItem.next() always returns an Object -- you need to make sure length > 0;

Also, Brackets requires tabbing of 4 spaces and passing JSLint (which you can turn on using View > JSLint).

Thanks,
Randy

@jeffslofish
Copy link
Contributor Author

Randy, thanks for the feedback. I have done a little testing with your solution, and I think there is still a bug. Here are the steps:

  1. Open 2 files in working set
  2. Make second file dirty
  3. Click the X on the first file

Results: Even though the mouse will be hovering over the second file, it will not have an X, but rather the dirty file icon, until you move the mouse.

I assume that it should also show an X in this case too, right?

@redmunds
Copy link
Contributor

Good catch! Need to determine canClose for the original file (not next file) since that's where next file will be after delete:

var canClose = ($listItem.find(".can-close").length === 1);

I thought of another case to also test:

  1. Open 2 files in working set
  2. Make second file dirty
  3. Select the first file
  4. Move mouse over file name (but not over X icon) so X appears for first file
  5. Press Cmd-W to close file

@redmunds
Copy link
Contributor

redmunds commented Nov 2, 2012

Jeff,

Can you submit these changes so I can merge this into master?

Thanks,
Randy

@redmunds
Copy link
Contributor

redmunds commented Nov 2, 2012

Looks good. Merging.

redmunds added a commit that referenced this pull request Nov 2, 2012
@redmunds redmunds merged commit a3178e9 into adobe:master Nov 2, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants