search

DennisSnijder / MakeGithubGreatAgain

Extension for making GitHub great again
https://chrome.google.com/webstore/detail/makegithubgreatagain/gpejlkhibgecggplgogpbgbdpnhogmhk
1.05k stars 49 forks source link

Github broke extension again #44

Closed thepixelmonk closed 7 years ago

thepixelmonk commented 7 years ago

Looks like the black bar is back :confused:

mscdex commented 7 years ago

I was just about to post this.... it looks like they're now nesting the header. The selector should now instead be something like body > div > .header or perhaps body div.header.

Andrews54757 commented 7 years ago

Its body > div > .header (https://developer.mozilla.org/en-US/docs/Learn/CSS/Introduction_to_CSS/Combinators_and_multiple_selectors)

jackwilsdon commented 7 years ago

Would it not be better just making it .header, effectively future proofing this from changes in the HTML layout?

thepixelmonk commented 7 years ago

Perhaps an extension option to customize the selector?

jackwilsdon commented 7 years ago

I'd say that absolutely referencing the element is not a good practice, as it's better to have "modular" CSS and nothing else on the page has the .header class.

jackwilsdon commented 7 years ago

How would that work exactly? The only way you can reliable detect the header is by using the .header class (or other classes pertaining to the header), so I don't see why we don't just go for .header on it's own.

jackwilsdon commented 7 years ago

If you publish using the Chrome Developer Dashboard, you can ignore this page.

And this extension is published using the dashboard, so I don't see what your point is?

Andrews54757 commented 7 years ago

Yeah, thats true. I forgot

Andrews54757 commented 7 years ago

Maybe another method to access .header should be used. GitHub could patch the extension by just renaming the class. However, certain things about the header are constant, they dont change.

jackwilsdon commented 7 years ago

I doubt they'll rename the .header class, it's a sensible name for it. They've kept it when doing the styling changes so I don't see why they'd change it again. I guess you could look it up by it's banner role too.

Andrews54757 commented 7 years ago

Mark Otto seems to be feisty about it though. (https://twitter.com/mdo/status/830138373230653440)

jackwilsdon commented 7 years ago

Yeah I get that, but the current class is sensible and hasn't changed even with all of the changes they've implemented. Having a more vague selector means we don't need to change the extension when (minor) HTML changes are made.

Andrews54757 commented 7 years ago

Okay. Also, maybe this project should port to stylus (https://chrome.google.com/webstore/detail/stylus/clngdbkpkpeebahjckkjfobafhncgmne). This is because GitHub is going to change more stuff that we might not like, and it would be harder for this project if it is a standalone extension.

Andrews54757 commented 7 years ago

https://github.com/StylishThemes/GitHub-Dark uses it

DennisSnijder commented 7 years ago

Fixed now, chrome extension store update coming soon!