-
-
Notifications
You must be signed in to change notification settings - Fork 449
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 a UI for uploaded images #4995
base: master
Are you sure you want to change the base?
Conversation
and all its template implementations
Found 2 issues while testing on 097eb7b:
Chatterino2_4995_097eb7b_WindowResizeRepaintingImages.mp4
|
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
// image, path, delete url | ||
|
||
UploadedImage UploadedImageModel::getItemFromRow( | ||
std::vector<QStandardItem *> &row, const UploadedImage &original) |
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.
warning: parameter 'row' is unused [misc-unused-parameters]
std::vector<QStandardItem *> &row, const UploadedImage &original) | |
std::vector<QStandardItem *> & /*row*/, const UploadedImage &original) |
|
||
img->pixmapOrLoad(); | ||
// You cannot stop me, clang-tidy | ||
auto *bleh = const_cast<ImagePtrItemDelegate *>(this); |
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.
warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
auto *bleh = const_cast<ImagePtrItemDelegate *>(this);
^
auto *bleh = const_cast<ImagePtrItemDelegate *>(this); | ||
bleh->ownedImages_[url] = img; | ||
} | ||
QSize sizeHint(const QStyleOptionViewItem &option, |
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.
warning: parameter 'option' is unused [misc-unused-parameters]
QSize sizeHint(const QStyleOptionViewItem &option, | |
QSize sizeHint(const QStyleOptionViewItem & /*option*/, |
bleh->ownedImages_[url] = img; | ||
} | ||
QSize sizeHint(const QStyleOptionViewItem &option, | ||
const QModelIndex &index) const override |
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.
warning: parameter 'index' is unused [misc-unused-parameters]
const QModelIndex &index) const override | |
const QModelIndex & /*index*/) const override |
Would it be possible to add the ability to bring up a on-hover pop-up when hovering over the image preview for a better view? Similar to when hovering over emotes and links in chat. |
I'm not sure I like this solution. It requires interaction to just look at an image, where there is no other good 'index' to go by. |
TODO
|
…re/image_uploader_ui
…re/image_uploader_ui
This diff is horrendous I am sorry.
This is ic_fluent_image_prohibited_24_regular from Fluenticons
This reverts commit a56d6b0.
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
//forget->setText("Remove image from log"); | ||
buttons->addStretch(); | ||
|
||
auto *view = layout.emplace<QListView>().getElement(); |
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.
warning: use of undeclared identifier 'QListView' [clang-diagnostic-error]
auto *view = layout.emplace<QListView>().getElement();
^
buttons->addStretch(); | ||
|
||
auto *view = layout.emplace<QListView>().getElement(); | ||
view->setViewMode(QListView::IconMode); |
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.
warning: use of undeclared identifier 'QListView' [clang-diagnostic-error]
view->setViewMode(QListView::IconMode);
^
view->setViewMode(QListView::IconMode); | ||
view->setModel(model); | ||
view->setIconSize(QSize(96, 96)); | ||
view->setLayoutMode(QListView::Batched); |
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.
warning: use of undeclared identifier 'QListView' [clang-diagnostic-error]
view->setLayoutMode(QListView::Batched);
^
view->setModel(model); | ||
view->setIconSize(QSize(96, 96)); | ||
view->setLayoutMode(QListView::Batched); | ||
view->setMovement(QListView::Static); |
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.
warning: use of undeclared identifier 'QListView' [clang-diagnostic-error]
view->setMovement(QListView::Static);
^
view->setUniformItemSizes(true); | ||
|
||
// without this prop qt throws everything into a single line | ||
view->setResizeMode(QListView::Adjust); |
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.
warning: use of undeclared identifier 'QListView' [clang-diagnostic-error]
view->setResizeMode(QListView::Adjust);
^
There were complexity complaints related to this PR. They mostly included removing the image preview. However UI-wise an implementation that doesn't load previews might as well just be your text editor. Searching for any image in upload history (for example to repost or delete) is already tedious enough, especially if it's older than a couple of days. IMO this idea is not worth doing without the preview. I'd like to hear feedback on continuing development of this. |
Description
This includes migrations which shouldn't be but might turn out being buggy, use at your own risk.
Ensure that have a backup of your
Logs/ImageUploader.json
file.This PR adds a UI that lists all logged images. It also adds a button that forgets an image from the log.
Preview as of 6223e34:
Known issues:
Ill be using
ic_fluent_image_prohibited_24_regular.svg
from Fluenticons