abe33 / atom-pigments

An Atom package to display colors in project and files.
MIT License
521 stars 68 forks source link

:bug: Fix marker background attachment #349

Closed JoryHogeveen closed 6 years ago

JoryHogeveen commented 7 years ago

Make sure color markers are always visible. Tested on Windows installation. Fixes: #348

JoryHogeveen commented 7 years ago

For consideration: The transparent (square pattern) background image could be set to background-size: 12px; to align it properly with the pigment markers. 6px looks nice too. Let me know if you want me to add it here, make a separate PR or don't change it at all.

Cxarli commented 7 years ago

LGTM. We don't have Windows CI, so I can't ask you to write any tests 😅

JoryHogeveen commented 7 years ago

Haha fair enough. But what tests would you want? It seems to me this background attachment would error on all systems (from my understanding of fixed positioning)

gilbertohasnofb commented 4 years ago

@abe33 @Cxarli I was still experiencing this issue in Linux but I can confirm this PR fixes it for me. Unfortunately there has been no new releases of pigments since 2017. Would it be possible for the devs to release this through apm so that users can get this fix (and other fixes made since then) through Atom itself?

Cxarli commented 4 years ago

@gilbertohasnofb I'm not sure whether I have permission to push to apm, and @abe33 hasn't been responsive since ever, so I'm afraid this is a dead end for pigments in general. Maybe we'll have to fork and republish at some point, but I have no stats on whether pigments is popular enough for it to go that effort.

gilbertohasnofb commented 4 years ago

@Cxarli thanks for the quick reply. It's really a pity about this package though.

I'm not sure whether I have permission to push to apm,

Would you mind giving it a try when you have a chance? I think apm uses GitHub credentials, so perhaps you will have permission since you are a collaborator with write access to this repo.

Maybe we'll have to fork and republish at some point, but I have no stats on whether pigments is popular enough for it to go that effort.

Well, pigments have over 3 million downloads and is nearly always listed in recommended packages for Atom. In fact, it's listed in the top three Google results for 'atom best packages 2020':

https://www.elegantthemes.com/blog/wordpress/best-atom-packages https://wpdatatables.com/best-atom-packages/ https://www.shopify.co.uk/partners/blog/best-atom-packages

I think that does suggest people still use this package a lot.

Cxarli commented 4 years ago

@gilbertohasnofb I think I just made a new release, could you please check?

gilbertohasnofb commented 4 years ago

@Cxarli first of all, thanks a lot for this.

About the new version, for some reason I am not able to update it through apm nor through Atom itself. I see that there is a new release here on GitHub and I also see that the build is passing on Travis Cl, but for some reason the new version is not showing for me (from my experience, this normally takes almost no time at all). Looking into the current package.json file, I see that the version is still set to "version": "0.40.2". Did you use the apm publish patch command to publish the package? The publish command normally takes care of version number and will increase it automatically as well as create the release on GitHub. See https://flight-manual.atom.io/hacking-atom/sections/publishing/

gilbertohasnofb commented 4 years ago

So I think the issue is that you have released a new version, but the version number is still set to 0.40.2 instead of 0.40.3 and so neither apm nor Atom recognise my current package as out-of-date, and users who are already using pigments won't receive a notification for update.

Cxarli commented 4 years ago

Ahh, I didn't use apm publish patch. I'll check the other things you said. Thanks for checking

gilbertohasnofb commented 4 years ago

No worries. Take a look at that documentation link, apm publish makes it super easy to automate this. In your case, you probably can just fix the json file and get away with it. Let me know once you had a look at it and I can test again. Thanks again for this new release!

Cxarli commented 4 years ago

Okay, it took a few tries and failures but it should work now. Can you confirm @gilbertohasnofb ?

gilbertohasnofb commented 4 years ago

Yes, it worked fine. I got the message that version 0.40.6 was available and I can confirm it is indeed the current version from this repo. Thank you so much and congrats for the release!

Cxarli commented 4 years ago

You're welcome - thank you for the notice and the help :smile: