-
-
Notifications
You must be signed in to change notification settings - Fork 455
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
Add custom image functionality for inline mod buttons. #5369
Conversation
- remove commented out code, - remove useless if
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
loadPixmapFromUrlLazy( | ||
(*item.getImage())->url(), [row](const QPixmap &pixmap) { | ||
row[Column::Icon]->setData(pixmap, Qt::DecorationRole); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should ensure the reference to row
remains valid. Furthermore, setData
should be called on the GUI thread.
if iconPath is present, getImage will always be valid.
This should be noted in a comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we can even check if row
is valid. I can't make it a smart pointer without a large refactor. I could look up the row by some kind of unique temporary ID, but it seems like a hack.
src/util/LoadPixmapLazy.cpp
Outdated
if (!reader.canRead() || reader.size().isEmpty()) | ||
{ | ||
qCWarning(chatterinoSettings) | ||
<< "Can't read mod action image at" << url.string << ":" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- This mentions a mod action image, but the function is more general (it can read any image).
- This will add a space between the URL and the colon (same as below).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does add a space, to be honest I don't care that much about that space.
d59967c
to
d9ff6ca
Compare
- now uses QPointer to ensure the view is valid - now calls the code on the main thread to never hit an assertion
…re/inline-mod-button-custom-images
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
i hate naming things
with their icon & line
chatterinoImage
This also cleans up ModerationAction code