Closed benomatis closed 4 years ago
Let me review and correct the errors.
Should be complete now, waiting for your review. Thanks! ;)
Just one remark, do you know why the circle around the button shows like this on click? It looks like it has more
width
rather thanheight
.
I copied the CSS from CompleteDiffButton
; the reason you cannot see it there is because it disappears immediately. I'll fix it on the other one too to stay consistent, if that's ok with you.
@benomatis: I copied the CSS from CompleteDiffButton, the reason you cannot see it there is because it disappears immediately. I'll fix it on the other one too to stay consistent, if that's ok with you.
Yeah, sounds good, as long as the the circle around the button shows in a correct shape I'm good with it :)
Yeah, sounds good, as long as the the circle around the button shows in a correct shape I'm good with it :)
Turns out it's the shape
param that's set to circle
. According to the doc there are 2 options, either circle
or round
, I tried round
but that makes it even wider, if I omit it, it makes it square. I'll play with the css a bit to make it really a proper circle, as it seems that's what you're after.
Ok, I just did that. I collected some default CSS rules shared between the 2 buttons (CompleteDiffButton
and this new one) so they can be set in a more consistent manner.
I think we are good to go here. Lucas' concern about the circle, my concern about the text, and (I'm assuming the name 😅) Ben's concern about formatting are all solved!
Thanks for the PR! 🙌
Sooo, it turns out I got too trigger-happy and merged early!
There are two console.log
s that could be removed, and @lucasbento found a bug:
@benomatis Could you make these changes in a new PR?
Sorry to both of you for the inconvenience.
... and I hoped I won't have to fiddle around with CSS. Until now, what I found out is that the attribute ant-click-animating-without-extra-node
seems to set itself to true
randomly, and when it does, you get the Copied!
text jumping. I'll try to fix it and create a new PR.
... and I hoped I won't have to fiddle around with CSS. Until now, what I found out is that the attribute ant-click-animating-without-extra-node seems to set itself to true randomly, and when it does, you get the Copied! text jumping. I'll try to fix it and create a new PR.
@benomatis: thank you! I think this happens mostly when you click on it before the popover shows, perhaps you can try to check if it was clicked before it shows the popover so the content doesn't resize.
@benomatis: thank you! I think this happens mostly when you click on it before the popover shows, perhaps you can try to check if it was clicked before it shows the popover so the content doesn't resize.
Yes, that seems to be the issue: it happens if you click too fast, before the default text fully shows.
Would this be too "quick and dirty"? I basically fixed the width of the tooltip - I'd understand why you would not prefer this option, just checking... :)
I honestly wouldn't mind that, @pvinis any thoughts on this?
What's the difference? 😬 We just don't have the blue bubble? I'm fine with that, yea.
@pvinis The difference is a slightly larger left and right padding when the "File path copied!" text is showing. Sorry, what blue bubble? :)
Oh, cool. Never mind, all good, I'm just blind, I didn't notice the difference! If it works then great. Make a new PR when you can, and we can try it ourselves there! :)
Summary
Adding a button to copy the file path to clipboard makes it easier to search for the file (eg. Command + Shift + O in JetBrain's IDE's) and make the necessary modifications. This is similar to the feature GitHub has.
Note that I'm using the
oldPath
going out from the assumption people will want to search for the file in the old version, and then do the modification to that.What are the steps to reproduce?