better-featured-project code cleanup
#8828
Conversation
DNin01
added
scope: addon
mxmou
left a comment
mxmou
left a comment
There was a problem hiding this comment.
Dynamic enable/disable works and some of the code changes look good, but there are also some I disagree with, especially those that look like arbitrary style decisions. Because we don't have a project-wide style guide, I don't see a reason not to stick to the style used by the original addon author unless the change significantly improves the code (makes it more readable, which isn't necessary the same thing as making it shorter or simpler).
In particular, I don't think element IDs should be abbreviated: it makes them easier to write but harder to understand, so it's only worth it if the abbreviation is used in many places. I'm also not convinced that replacing querySelector() with getElementById() and similar changes are justified.
| document.querySelector("#fpb-name h3").innerText = featuredTitle; | ||
| } | ||
| if (document.querySelector('#featured-project [data-control="edit"]') !== null) { | ||
| if (document.querySelector('#featured-project [data-control]')) { |
There was a problem hiding this comment.
This is very minor, but I'd keep the ="edit" part: it has no impact on the specificity or behavior, but I think it makes the selector a little more self-explanatory.
2 participants
Changes
The Profile Page Banner addon has been updated. The code is now easier to read, and the content now loads earlier.
Reason for changes
Before, the userscript did not have
"runAtComplete": false, so the featured project image could take several seconds to appear, particularly on profiles with a lot of comments, due to images having to load first.Notes
Tested in Edge 144. Works with
live-featured-project.Make sure to check the Commits tab to understand the changed code.
"fpb" stands for "featured project banner", btw.
@mxmou, you can review this PR if you want—you updated this addon recently.
To help with testing with the membership badge, I've also made
test-membership.