Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(114)

Unified Diff: chrome/browser/ui/views/download/download_item_view.cc

Issue 2556573002: Fix SizeLabelToMinWidth() function such that no unnecessary space (Closed)
Patch Set: nit Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/ui/views/download/download_item_view.cc
diff --git a/chrome/browser/ui/views/download/download_item_view.cc b/chrome/browser/ui/views/download/download_item_view.cc
index 99b5344d90e8e6a49eaaed4000c68bd01650753a..32a6e0aecb50607b4b660f07d256e4510eb6d8ab 100644
--- a/chrome/browser/ui/views/download/download_item_view.cc
+++ b/chrome/browser/ui/views/download/download_item_view.cc
@@ -1015,6 +1015,15 @@ gfx::Size DownloadItemView::GetButtonSize() const {
void DownloadItemView::SizeLabelToMinWidth() {
if (dangerous_download_label_sized_)
return;
+ gfx::Size size = dangerous_download_label_->GetPreferredSize();
+ // If the size of the label already smaller than kDangerousDownload, it can
+ // fit into a single line. This is to prevent a short string (e.g.: "This file
+ // is malicious") from being broken up unnecessarily.
Peter Kasting 2016/12/06 05:48:20 Nit: Grammar and constant naming problems. How ab
Jialiu Lin 2016/12/06 23:42:33 Ah.. auto complete.. Thanks for catching this.
+ if (size.width() <= kDangerousTextWidth) {
+ dangerous_download_label_->SetSize(size);
+ dangerous_download_label_sized_ = true;
Peter Kasting 2016/12/06 05:48:20 Nit: Instead of doing this here, set it immediatel
Jialiu Lin 2016/12/06 23:42:33 There are actually two things to be set in this fu
Peter Kasting 2016/12/08 01:05:56 Return a tuple? Return the text to use, and recal
Jialiu Lin 2016/12/08 04:51:58 Done.
+ return;
+ }
base::string16 label_text = dangerous_download_label_->text();
base::TrimWhitespace(label_text, base::TRIM_ALL, &label_text);
@@ -1036,15 +1045,14 @@ void DownloadItemView::SizeLabelToMinWidth() {
DCHECK(status);
base::string16 prev_text = original_text;
- gfx::Size size = dangerous_download_label_->GetPreferredSize();
- int min_width = size.width();
+
Peter Kasting 2016/12/06 05:48:20 Nit: Use at most one blank line at a time within a
Jialiu Lin 2016/12/06 23:42:33 Done.
+
+ gfx::Size min_width_size = size;
// Go through the string and try each line break (starting with no line break)
- // searching for the optimal line break position. Stop if we find one that
- // yields one that is less than kDangerousTextWidth wide. This is to prevent
- // a short string (e.g.: "This file is malicious") from being broken up
- // unnecessarily.
- while (iter.Advance() && min_width > kDangerousTextWidth) {
+ // searching for the optimal line break position. Stop if we find one that
+ // yields minimum label width.
+ while (iter.Advance()) {
size_t pos = iter.pos();
if (pos >= original_text.length())
break;
@@ -1061,13 +1069,15 @@ void DownloadItemView::SizeLabelToMinWidth() {
size = dangerous_download_label_->GetPreferredSize();
// If the width is growing again, it means we passed the optimal width spot.
- if (size.width() > min_width) {
+ if (size.width() >= min_width_size.width()) {
Peter Kasting 2016/12/06 05:48:20 Changing this to >= seems worse. The effect is th
Jialiu Lin 2016/12/06 23:42:33 Agree, change to '>' will place as much text on th
dangerous_download_label_->SetText(prev_text);
- break;
+ dangerous_download_label_->SetSize(min_width_size);
+ dangerous_download_label_sized_ = true;
+ return;
} else {
Peter Kasting 2016/12/06 05:48:20 Nit: No else after return
Jialiu Lin 2016/12/06 23:42:33 Done.
- min_width = size.width();
+ min_width_size = size;
+ prev_text = current_text;
}
- prev_text = current_text;
}
dangerous_download_label_->SetSize(size);
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698