Skip to content

Blogs comparator handles empty titles properly#2349

Closed
solinger10 wants to merge 1 commit intowordpress-mobile:developfrom
solinger10:issue/2328-blog-title-sorting
Closed

Blogs comparator handles empty titles properly#2349
solinger10 wants to merge 1 commit intowordpress-mobile:developfrom
solinger10:issue/2328-blog-title-sorting

Conversation

@solinger10
Copy link
Copy Markdown
Contributor

Fixes #2328. The comparator used the url to compare blogs with no titles to blogs with titles. This results in blogs with no title being put in the 'h' section (for 'http://...') in a sorted list. I changed it to only use the url for comparing two blogs if both blogs don't have a title.

@maxme maxme self-assigned this Feb 27, 2015
@maxme maxme added this to the 3.9 milestone Feb 27, 2015
@maxme
Copy link
Copy Markdown
Contributor

maxme commented Feb 27, 2015

I think we should sort blogs with an empty title by their "hostname". Here is my proposed patch for BlogNameComparator:

         public int compare(Object blog1, Object blog2) {
-            Map<Object, Object> blogMap1 = (Map<Object, Object>) blog1;
-            Map<Object, Object> blogMap2 = (Map<Object, Object>) blog2;
-
-            String blogName1 = MapUtils.getMapStr(blogMap1, "blogName");
-            if (blogName1.length() == 0) {
-                blogName1 = MapUtils.getMapStr(blogMap1, "url");
-            }
-
-            String blogName2 = MapUtils.getMapStr(blogMap2, "blogName");
-            if (blogName2.length() == 0) {
-                blogName2 = MapUtils.getMapStr(blogMap2, "url");
-            }
-
+            Map<String, Object> blogMap1 = (Map<String, Object>) blog1;
+            Map<String, Object> blogMap2 = (Map<String, Object>) blog2;
+            String blogName1 = getBlogNameOrHostNameFromAccountMap(blogMap1);
+            String blogName2 = getBlogNameOrHostNameFromAccountMap(blogMap2);
             return blogName1.compareToIgnoreCase(blogName2);
         }

@maxme
Copy link
Copy Markdown
Contributor

maxme commented Feb 27, 2015

screencap-2015-02-27t11-10-09 0100

@maxme
Copy link
Copy Markdown
Contributor

maxme commented Mar 4, 2015

@solinger10 What do you think about the proposed solution?

@solinger10
Copy link
Copy Markdown
Contributor Author

I don't really like the blogs with no title being compared to all other blogs using the host name. It just seems to make more sense to me to have the no title blogs at the top of the list, but that's all personal preference. Both options solve the problem.

@maxme
Copy link
Copy Markdown
Contributor

maxme commented Mar 5, 2015

I don't really like the blogs with no title being compared to all other blogs using the host name.

I agree, on this screen it's not the best UX. But we're using that comparator in other places like here:

screencap-2015-03-05t11-03-29 0100

(taliwuttandfood2.wordpress.com has an empty title)

Where we only show blog titles or blog url if the title is empty, in that case that's weird to have it on top. We could have different blog comparators, but IMO 2 blog lists with 2 different orders is just another issue.

@solinger10
Copy link
Copy Markdown
Contributor Author

Yeah, I see what you mean. I think it looks worse there and not so bad in comparison on the other list. I'd say your patch is the way to go.

@maxme
Copy link
Copy Markdown
Contributor

maxme commented Mar 6, 2015

ref #2403

@maxme maxme closed this Mar 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Blogs with no title show in unexpected spot in show/hide blogs view

2 participants