Skip to content
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

[#37] Bundle emails and link to labels #68

Merged
merged 49 commits into from May 3, 2019

Conversation

abasiri
Copy link
Contributor

@abasiri abasiri commented Apr 30, 2019

[#37] This makes an attempt at introducing bundles by linking to search for "in:inbox label:"

There are still many features missing, and likely some bugs like:

  • Ability to turn on and off using extension options
  • Highlighting as read/unread depending on emails being bundled.
  • Will entry correctly disappear if underlying emails are archived
  • Potential other bugs.

image

@russelldc
Copy link
Collaborator

russelldc commented Apr 30, 2019

To anyone wanting to test this with the default categories, as shown in @abasiri 's screenshot, you'll need to click the dropdown next to each category and allow it In message list:

Screen Shot 2019-04-30 at 2 05 50 PM

Awesome start, found a few issues:

  1. The title of the bundle is oddly spaced to the right. In your screenshot, this looks alright, because of your specific screen width and because you have the Important tags on. But on mine, it's odd. You can see that in the screenshot below.
  2. I'm seeing the date bundle headers show up with no messages below them. This happens when an email was in your inbox from Last Month, for example, but it was bundled by label up above.
  3. Shouldn't label bundles be separated by date as well? I need to do a little research, I can't remember if that's how it worked in Inbox or not.
  4. There's something going VERY wrong with the emails that get left behind in the inbox. I'm clicking on emails in the list and it's sending me to entirely different ones.
  5. Other times, clicking on either emails or the bundle does nothing at all. When I look in the console, I'm seeing errors like below, logged when I click on items in the email list:
m=b:452 Uncaught rWa {message: "Error in protected function: Cannot read property 'Fy' of undefined", stack: "TypeError: Cannot read property 'Fy' of undefined↵…s=AHGWq9ALJkiXFaqbqcxNHCNbW0ftvhvFvw/m=b:542:101)"}

Screenshot of how the bundle title offset it looks on my end:

Screen Shot 2019-04-30 at 2 11 19 PM

Questions:

  1. Do you plan to include the count?
  2. Do you plan to show the list of senders?

Here's a screenshot of Inbox from March 2019, showing those 2 features:

inbox

No need to show attachments and all that junk, I actually wasn't a fan of that in the original Inbox, anyway.

Suggestions / Future Enhancements

  1. If we're going with redirecting to the label search page rather than opening inline, then I think it would be greatly improve UX to add a back button.
  2. It'd be nice to include the official colored titles as shown in that screenshot, as well as custom label colors, but those can be future tasks after this initial work.
  3. Also it's not needed for this PR, but the label bundles could have a sweep button over to the right, which would solve issue sweeping #44.

@russelldc
Copy link
Collaborator

russelldc commented Apr 30, 2019

Trying to figure out the 4th issue I listed above...
It seems like if there's anything bundled, if you click a non-bundled email below the bundle, you are sent to the email just below it, instead, like an indexing problem. So if you click the last email in the list, nothing happens, just an error.

@abasiri
Copy link
Contributor Author

abasiri commented Apr 30, 2019

I agree that there are a ton of issues with this. it seems that the way gmail handles click events is tied to email positions rather than event targets, which means when emails get bundled, clicking on emails can open up the wrong emails. I spent sometime on this to no avail.

Also the same goes for keyboard shortcuts, if you have them on, lots of errors are thrown as the emails they expect are no longer there.

@russelldc
Copy link
Collaborator

I'll try seeing if I can figure anything out, later tonight.

@abasiri
Copy link
Contributor Author

abasiri commented Apr 30, 2019

I think i found the culprit. They are using index of <tr>. Since I'm adding a <tr> element, it throws everything off. Converting it to a <div> with some styling should make it work hopefully.

@russelldc
Copy link
Collaborator

russelldc commented Apr 30, 2019

By the way, the 'odd spacing' between the icon and title was just that this extra element was present,
<td class="WA xY"></td>, even when Important markers were turned off.

Here's one way to tell if important markers are turned on:
if (document.querySelector('.WA.xY')​) ...

A naive solution:

...
(document.querySelector('.WA.xY') ? "	<td class=\"WA xY\"></td>" : "") +
...

@russelldc
Copy link
Collaborator

russelldc commented Apr 30, 2019

I tried switching the <tr> to <div>, but unfortunately, among other things, the label-link element gets lost somehow?

Edit:

Oh! Got it, every single child element needs to be turned into a <div> instead of <td>s as well. Seems to work now, just some very minimal style adjustment needed, and doesn't get the indexing issue.

Edit 2:

Here's the styling changes:

.bundle-wrapper {
	padding-left: 20px !important;
}

.bundled-email {
	display: none !important;
}

.bundle-image {
	width: 28px;
	padding-right: 3px;
}

Now there's just the issue of separating those bundles by month.

Edit 3:

Agh, of course, this looks weird right now, if you have Important marker turned on :P
Although, it's not that bad, in my opinion, if you just completely remove the <div class="WA xY"> element. It differentiates the bundle more from a regular email.

@russelldc
Copy link
Collaborator

russelldc commented Apr 30, 2019

Here's some ideas on how to create links for the label bundles by time period:

To get today's Promotions, you'd search for:
in:inbox label:"Promotions" after:2019/4/30 before:2019/5/1

The URL would be:
'#search/' + encodeURIComponent('in:inbox label:"Promotions" after:2019/4/30 before:2019/5/1')
==>
#search/in%3Ainbox+label%3A"Promotions"+after%3A2019%2F4%2F30+before%3A2019%2F5%2F1

@jcgoble3
Copy link

So I'm testing this on Firefox on Ubuntu and can't get it to work. When the Gmail page first loads, I can very briefly see label bundles, but then the page immediately (within a fraction of a second) updates and all bundles go away, and I cannot see any way to get the bundles back. Is this an Ubuntu issue or a Firefox issue? Am I doing something wrong? If I knew how to record my screen, I'd create a GIF or a short video illustrating this, but I don't know how.

Additionally, there are several display overflow issues. None of the items on the top bar, when expanded (e.g. search bar dropdown, items in the upper right), extend past the bottom of the blue bar (screenshots: search bar, upper right), and the items on the right clobber the entire top bar, leaving it blank when closed (screenshot) and requiring a page reload to restore it to normal. Also, the label color submenu on the labels on the left side is hidden past the edge of the left sidebar (screenshot). Again, these may be either an Ubuntu issue and/or a Firefox issue; I am not sure what combinations you have tested on.

The glimpse I get of the bundles during page loading looks promising, so I hope we can get this to work.

@russelldc
Copy link
Collaborator

@jcgoble3 The second thing you mention is a Firefox specific problem -- there was some css introduced by @boukestam that was supposed to fix some problem he saw in Firefox, but everyone who's tested it said that that css actually causes the problem. To fix that, just remove lines 1000-1016:

/* Firefox-specific fixes */
@-moz-document url-prefix() {
	/* overflow fix emails list */
	.AO {
		overflow-y: auto;
	}

	/* overflow fix left sidebar */
	.oy8Mbf {
		overflow-y: hidden;
	}

	/* overflow fix right sidebar */
	.brC-aT5-aOt-bsf-Jw, .brC-bsf-aT5-aOt {
		overflow-y: hidden;
	}
}

@abasiri Could you remove that from this branch?

@russelldc
Copy link
Collaborator

@jcgoble3 That's interesting, I haven't seen the bundles appear then disappear later. I doubt it would be an OS problem. I'll try this branch now in Firefox (macOS).

@russelldc
Copy link
Collaborator

russelldc commented Apr 30, 2019

@jcgoble3 I'm on Firefox v66 on macOS, and the bundling is working fine for me. Could you please try my branch? It has the changes I described in comments above (except the date bundling). It should be a lot less glitchy.

https://github.com/boukestam/inbox-in-gmail/tree/ali-bundle-link-to-labels
https://github.com/boukestam/inbox-in-gmail/archive/ali-bundle-link-to-labels.zip

@jcgoble3
Copy link

@russelldc Just tested that. None of the CSS overflow issues are present in yours, so that's good. Bundles appear fine and show the correct emails when clicked.

However, the emails in the bundles still also show up separately as individual lines in the main inbox, which at least for me defeats the purpose. Bundles for me serve two purposes: grouping related emails together for easy reference, as well as getting those emails out of the way so the rest of my inbox can be easily used. Instead, most of my inbox is emails that are also in bundles. Ideally, an email should appear either in a bundle or in the main inbox, but not both (as in the defunct Inbox).

Compare my current actual Inbox tab (screenshot) with your extension in Gmail (page 1, page 2). The former is easy to navigate, the latter is extremely cluttered. Getting the bundled emails out of the way is actually more important to me than the bundles themselves, because having substantially fewer lines makes the occasional critically important email easier to spot. I would suspect this could be done with some JS code that applies "display: none" CSS to each line containing a bundled label.

Speaking of which, it also seems to create a bundle for every label in the inbox (notice the Amazon bundle, which is a label I do not bundle in Inbox), which I don't want (particularly if the bundled emails are removed from the main list). There are some labels that I apply automatically via filters for purposes other than bundling, and I still want those emails to appear in the main list unbundled because they are generally important and often need immediate attention (such as Amazon). The actual Inbox allows me to choose which labels I want bundled and which ones I don't. A similar option (even if I have to dig into Firefox's add-ons settings to configure it) would be nice here.

P.S. I'm not intending to come off as overly demanding here, so apologies if I'm coming off this way. I'm just trying to offer some constructive feedback based on my workflow.

@jcgoble3
Copy link

On a second look, the extension does remove the last couple of days of bundled email from the main list, but older emails are still shown individually. This only lasts for a short time (a few seconds) until Gmail next updates, upon which all emails are shown in the main list.

@russelldc
Copy link
Collaborator

@jcgoble3 That's so weird... they definitely stay only in their bundle, and don't show up in the list for me.

@russelldc
Copy link
Collaborator

russelldc commented Apr 30, 2019

@jcgoble3 In your screenshot, I notice there's a lot of missing first letter avatars on the emails that are escaping the bundles. Could you please check the web console to see if there's any errors?

If you're not familiar, you can get to it in Firefox with Ctrl+Shift+J.

@jcgoble3
Copy link

Screenshot of the console: https://snag.gy/tVwfiN.jpg

All of the recent emails (last ~2 days) show up with the first letter avatars on initial page load. After a few seconds (simultaneously with the bundled emails reappearing), most of those avatars disappear.

@russelldc
Copy link
Collaborator

russelldc commented Apr 30, 2019

@jcgoble3 I think most of that is "normal", as in it shows up without the extension.

The ones that stand out are the 3 Empty string passed to getElementById(). That seems relevant.

Edit: Although, I just checked, and none of the code in the extension explicitly calls "getElementById"

@jcgoble3
Copy link

jcgoble3 commented May 1, 2019

I am now home and have checked both versions in Firefox for Windows 10. Behavior of both is virtually identical to Ubuntu, with the sole difference being that under @russelldc 's version, the missing avatars/icons also include all icons in the left sidebar and the snooze, archive, and star hover actions on the right side of each row (the other two hover actions show up fine).

Also, I clicked the source code link to the right of the Empty string passed to getElementById(), and that leads to Google's own code, which is minified. After further investigation with the console open, I was able to reliably (on Windows, not tested on Ubuntu) trigger the warning on demand by passing the mouse cursor over the buttons on the right side of the page (namely Compose, Reminder, Calendar, Tasks, and Keep). Moving the mouse cursor repeatedly from one of those to another triggered several dozen copies of the warning in under a minute.

I then deleted the extension and reloaded the page, and the Calendar, Tasks, and Keep buttons in Google's own interface (but not the native Compose button in the upper left) also consistently triggered the warning on mouseover. So it appears to be a problem in Google's code that is somehow inherited by your Compose and Reminder buttons.

@russelldc
Copy link
Collaborator

@jcgoble3 missing all of the icons is likely due to my most recent commit. It was a change that loaded all images locally from the extension, instead of relying on a 3rd party image host. At one point we were using Google's original image links for those icons, then got anxious that they'd eventually stop working since Inbox was shut down, so @g60madman uploaded them to an image hosting site. Thanks a lot for pointing this out, I didn't test out that change in Firefox.

So far I've tested in Firefox macOS (before the image change), Chrome macOS and Chrome on Windows 10, and had no issues.

I'll try Firefox on Windows 10 later today... so far I'm thinking the unbundling problem you're encountering has nothing to do with browser/OS, but something to do with your specific set of emails. I'll try re-adding a bunch of old emails to my Inbox for testing.

Just in case, could you please try this in Chrome?

@russelldc
Copy link
Collaborator

russelldc commented May 1, 2019

Just tried it out on Firefox v66 on Windows 10, and besides the missing icons issue I need to fix, the bundling is working the same as it does on Chrome for me. So I'm still thinking it has something to do with your specific collection of emails -- I'll try filling up my inbox with a bunch, old and new, to see what happens.

@russelldc
Copy link
Collaborator

Replying to comment left by @jcgoble3 in abasiri/inbox-in-gmail#1

In one of my commits, I'd changed the fixLabel function to not just apply to all non-alphanumeric, because labels with periods in them were not working at all, and labels with spaces in them were getting users stuck in a redirect loop.

@abasiri Does the new regex work for all situations? Examples:

  • TheForce.net (the label that was breaking with the first version, for @jcgoble3)
  • The Force.net
  • Parent/Child
  • Parent Label/Child Label
  • Groceries&Shopping
  • Groceries+Shopping

@russelldc
Copy link
Collaborator

russelldc commented May 3, 2019

Updated TODO

Fixes

  1. Test that all of the above labels work with the latest commit
  2. When de-duping senders, if there are multiple emails from the same person and any are unread, then show as bold, if all are read then show as normal text. (reported by @jcgoble3, strategy by @abasiri)
  3. For a conversation with multiple participants, this extension always chooses the sender of the first message to display in the bundle author list. Inbox, however, displays the sender of the last message for read conversations and the sender of the first unread email for unread conversations, which is more useful. (reported by @jcgoble3)
  4. Fix bundle flashing issue: insert a <style> tag and add the id of the email, then you can hide it properly. That way when gmail rewrites the tag for the emails and we lose the classes on it, it won't matter, it'll still be hidden. (strategy by @abasiri) For example:
<style>
  [id=":qw"] {
    display: none;
  }
</style>

Enhancements

  1. Reduce the opacity of a bundle's icon image if bundle contains no unread messages. As requested by @abasiri and @jcgoble3: I find that I want that to kind of fade to distinguish it from the unread ones better.
  2. Display unique icons (and respective font color for the label-link) for Gmail's built-in categories (Promotions, Updates, etc.) as seen in this screenshot of Inbox. @jcgoble3 fetched the images from Inbox for us: inbox-images.zip.
    Edit: I just realized I had these images on my desktop all this time, since the time I'd tried to save Inbox's html/css/js before shutdown. I'd apparently just never added these particular icons to this repo. And even better news, I had grabbed the 2x res versions, @jcgoble3's zip has the lower res defaults. I'll go add my collection to the master branch, now.
  3. For custom labels, use their label color for the text color of the bundle's label-link and icon (it's an image, but might be possible with some CSS hackery).

The functionality listed below would likely require Gmail.js for reliable observers/event listeners:

  1. Display a sweep all emails icon button on bundle row on mouse hover, clicking this would archive all emails contained in bundle that aren't pinned
  2. Display a back button inside the bundle (search page), to return to the inbox
  3. Clicking done on an open bundled email should return you to the bundle (search page)
  4. If inside an empty bundle (search page), it should return you to the inbox (including if you clicked Done on an email in the list)

@russelldc
Copy link
Collaborator

Even if only fixes 1 & 4 get done right now, I'd consider it ready to merge and publish (although I just saw that issue @boukestam posted... 😞). The others could all be moved to a new issue for enhancing the bundles.

@abasiri
Copy link
Contributor Author

abasiri commented May 3, 2019 via email

@russelldc
Copy link
Collaborator

@abasiri Broken links would definitely affect productivity :P

I just tested the problem I described in Fix 1 (label link regex), and at least one old bug that I fixed has returned. Once you click on a label with a space in it, you can't use the back button anymore.

@abasiri
Copy link
Contributor Author

abasiri commented May 3, 2019

@russelldc ah, sorry I was accidentally looking at 1 & 4 under enhancements.

@abasiri
Copy link
Contributor Author

abasiri commented May 3, 2019 via email

@russelldc
Copy link
Collaborator

The latest commit does not fix the issue I was describing with spaces. I still can't use the back button with this label:
Inbox In Gmail

@abasiri
Copy link
Contributor Author

abasiri commented May 3, 2019

looking.

@russelldc
Copy link
Collaborator

russelldc commented May 3, 2019

@abasiri This works for the label Inbox In Gmail, but I haven't tried it with the entire list of labels yet:

.replace(/ /g, '+') instead of .replace(' ', '+')

@abasiri
Copy link
Contributor Author

abasiri commented May 3, 2019

Looks like replacing space with "-" is the correct way. I haven't tried the other chars either. I'm looking at all the chars next.

label.replace(/[\/\\ ]/g, '-'))

@russelldc
Copy link
Collaborator

russelldc commented May 3, 2019

Tested latest commit

Broken:

  • Groceries&Shopping - partly broken, doesn't get stuck in back button loop, but adds an extra page to history that you need to back out of, when the & gets converted to %26
  • Groceries & Shopping - partly broken, doesn't get stuck in back button loop, but adds an extra page to history that you need to back out of, when the & gets converted to %26
  • Groceries+Shopping - broken, no results. Native gmail does this search to get it to work: label:groceries+shopping
  • Groceries + Shopping - broken, no results. Native gmail does this search to get it to work: label:groceries-+-shopping
  • [Test] - broken, no results. Native gmail does this search to get it to work: label:[test]

Works:

  • Groceries-Shopping
  • Groceries - Shopping
  • TheForce.net
  • The Force.net
  • The Force .net
  • The Force . net
  • Parent/Child
  • Parent Label/Child Label

@russelldc
Copy link
Collaborator

russelldc commented May 3, 2019

Updated my testing list in the above comment.

Half of those that are broken get fixed by just not wrapping the label with quotation marks.

@abasiri
Copy link
Contributor Author

abasiri commented May 3, 2019

Yeah between the quotes and using component encode, it fixed all the issues.

@abasiri
Copy link
Contributor Author

abasiri commented May 3, 2019

Found another issue though, if the label is too long it overflows onto the subject.

@abasiri
Copy link
Contributor Author

abasiri commented May 3, 2019

I'm gonna be away from this for the rest of the day.

@russelldc
Copy link
Collaborator

russelldc commented May 3, 2019

@abasiri How long is the label when that happens? I'm not seeing that.

Also, what does overflow mean in this context, is the text escaping the rectangle bounds of the label?

@russelldc
Copy link
Collaborator

I'll merge this now, comment later when you get a chance to explain the overflow issue, a screenshot would be great

@russelldc russelldc removed the do not merge This code isn't ready to publish label May 3, 2019
@russelldc russelldc merged commit a36cbc0 into boukestam:master May 3, 2019
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.

None yet

3 participants