The Wayback Machine - https://web.archive.org/web/20220506192149/https://github.com/atom/status-bar/pull/118
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.

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

Make shift clicks on the currentPath element copy the relative path #118

Merged
merged 3 commits into from Feb 8, 2016

Conversation

Copy link
Contributor

@awans awans commented Jan 30, 2016

Implement ctrl/cmd click -> copy relative path as discussed in this issue.

I've added the most obvious test, but let me know if there are others you'd like me to take a pass at..

Please also let me know if I've missed any subtle issues!

This is my first:

  • atom contribution
  • coffeescript
  • atom spec

so I'd love to learn :)

clickHandler = =>
text = @getActiveItemCopyText()
clickHandler = (event) =>
altClick = if process.platform is 'darwin' then event.metaKey else event.ctrlKey

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this need to differ by platform? Can't you just make it Ctrl for all platforms?

Copy link
Contributor Author

@awans awans Jan 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, but I'd rather not from a UX perspective -- control clicking on OS X is a lot less natural than Command clicking; this is the pattern that Facebook's hyperclick package uses, too!

Copy link
Member

@50Wliu 50Wliu Jan 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine the way it is right now - cmd is equivalent to ctrl on Windows/Linux.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cf #104 (comment) for my feelings on it

@lee-dohm lee-dohm added the atom label Feb 1, 2016
@thedaniel
Copy link

@thedaniel thedaniel commented Feb 5, 2016

@awans This looks pretty good - I think altClick is kind of a confusing variable name since alt is another modifier key, and it's not clear that it's a boolean. Anyway, I left a comment over on #104 with some concerns about the feature in general, but if those are resolved positively this probably only needs that minor change to merge.

@awans awans changed the title Make ctrl/meta clicks on the currentPath element copy the relative path Make shift clicks on the currentPath element copy the relative path Feb 5, 2016
@copiedTooltip = atom.tooltips.add this,
title: "Copied: #{text}"
trigger: 'click'
delay:
show: 0

getActiveItemCopyText: ->
getActiveItemCopyText: (isShiftClick) ->
Copy link
Contributor

@benogle benogle Feb 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rename this from isShiftClick to useRelativePath for clarity?

Copy link
Contributor

@benogle benogle Feb 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll change the name on my end

Copy link
Contributor Author

@awans awans Feb 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sgtm, and thanks!

benogle added a commit that referenced this issue Feb 8, 2016
Make shift clicks on the currentPath element copy the relative path
@benogle benogle merged commit 333a039 into atom:master Feb 8, 2016
1 check passed
@benogle
Copy link

@benogle benogle commented Feb 8, 2016

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment