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

Fix AdBlock blocking share icon, ref #866 #4337

Merged
merged 1 commit into from
Apr 13, 2017
Merged

Conversation

jancborchardt
Copy link
Member

This finally takes care of the share icon being blocked by AdBlock (or other adblocking plugins). As the user bases of Adblock and Nextcloud probably have some reasonable overlap, we should fix this.

This is simply achieved by renaming the class from icon-share to icon-shared, which doesn’t seem to be blocked.

Ref #866, please review @kumzugloom @Pommesumdreher @MariusBluem @Bullnados @GetBoz @schiessle

Before: share icon is missing, and with that you don’t really know where to click / how to share:
adblock share icon issue
After: it’s displayed, as intended – same as when you don’t have Adblock:
adblock share icon fix

Will also open pull requests to open this in other apps using the icons, like Calendar and Contacts.

Also, I would say this is a bugfix to backport – any objection @rullzer? ;)

Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
@jancborchardt jancborchardt added 3. to review Waiting for reviews backport-request bug design Design, UI, UX, etc. feature: sharing papercut Annoying recurring issue with possibly simple fix. labels Apr 12, 2017
@jancborchardt jancborchardt added this to the Nextcloud 12.0 milestone Apr 12, 2017
@mention-bot
Copy link

@jancborchardt, thanks for your PR! By analyzing the history of the files in this pull request, we identified @rullzer, @VicDeo and @blizzz to be potential reviewers.

@jancborchardt
Copy link
Member Author

Please also review the pull requests to Calendar nextcloud/calendar#417 and Contacts nextcloud/contacts#188 :)

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Tested and works 👍 Solution looks fine

@MorrisJobke MorrisJobke merged commit ec03475 into master Apr 13, 2017
@MorrisJobke MorrisJobke deleted the fix-adblock-share-icon branch April 13, 2017 17:10
@@ -326,14 +326,12 @@ img, object, video, button, textarea, input, select {
background-image: url('../img/actions/settings-white.svg?v=1');
}

/* always use icon-shared, AdBlock blocks icon-share */
.icon-shared,
Copy link
Member

Choose a reason for hiding this comment

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

This now uses a different image 👎 please fix

Copy link
Member Author

Choose a reason for hiding this comment

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

The images share.svg and shared.svg are and have always been exactly the same. What’s the issue exactly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug design Design, UI, UX, etc. feature: sharing papercut Annoying recurring issue with possibly simple fix.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants