Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Add filename to title attribute #609

Merged
merged 1 commit into from Nov 9, 2015

Conversation

johanderuijter
Copy link
Contributor

Fixes #557

I'm not sure if this will require any tests, if it does, i'm going to need some guidance. I checked the tree-view-spec and with it's 2.5k+ lines it's not really beginner friendly.

Let me know if any changes are needed :)

@billybonks
Copy link
Contributor

@benogle can we merge this ?

@mnquintana
Copy link
Contributor

Sorry for the delay on reviewing this!

I think this is a good change, but what do you think about adding this for directories too? A directory name could definitely also exceed the width of the tree view.

Also, we generally use the Atom Tooltip API for things like this, instead of native tooltips. What do you think about displaying tooltips that way?

@billybonks
Copy link
Contributor

@mnquintana created a version with the tooltip api take a look and let me know what you think #610

@johanderuijter
Copy link
Contributor Author

Added the title attribute to directories aswell.

I used the title attribute instead of the tooltips as they seemed less intrusive (and it was suggested in the original issue), but the other PR looks good to. I'm good with either.

@billybonks
Copy link
Contributor

Ye to use the tooltip api i would still need to

  • Create a setting that turns it off.
  • Only show tooltips if the name is hidden by the viewport.
  • fix the positioning bug.

@mnquintana
Copy link
Contributor

I think this is fine for now - it would be more consistent to use Atom tooltips, but also more finicky and obtrusive. We can always change this to Atom tooltips in a subsequent PR. Thanks for adding this!

mnquintana added a commit that referenced this pull request Nov 9, 2015
@mnquintana mnquintana merged commit 4c02c40 into atom:master Nov 9, 2015
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.

None yet

4 participants