InboxSDK
Bug: Cannot update/reset values via `replaceDate` or `replaceDraftLabel`
When calling replaceDraftLabel or replaceDate multiple times with different text values or when trying to reset to the original value of a thread row, the behavior is not as I would expect it.
- Resetting is not doing anything!
- Updating does not replace the value, but adds another
<span>, so that we end up with duplicate values.
Expected behavior
When passing null as the parameter to replaceDraftLabel or replaceDate, I would expect the corresponding values for the thread row to be reset.
When calling replaceDraftLabel or replaceDate repeatedly, I would expect the corresponding values for the thread row to be replaced with the latest text value.
Version
I am using the latest version of InboxSDK, installed/bundled via npm.
@fjaeger thanks for the report. Can you create a minimum reproducible example?
@wegry Sure! Please find the demo project here: https://github.com/Mailbutler/inboxsdk-row-bug-demo
@fjaeger thanks for the demo project. I can repro locally with it.
~~It looks like we might be inadvertently falling into this block. Gmail's DOM may have shifted since this code was written (the selector in use here is from 10 years ago).~~ edit: this might be a red herring.
For posterity's sake, with the following diff against 9dd4175d and the much older cc1f4f0c4da1314f9d5813a0a5b9193217612680, it looks like this issue wasn't introduced by recent changes to gmail-thread-row-view in #1148 or #1150.
diff --git a/examples/thread-rows/content.js b/examples/thread-rows/content.js
index 9123776b..0d0a3bca 100644
--- a/examples/thread-rows/content.js
+++ b/examples/thread-rows/content.js
@@ -23,188 +23,9 @@ InboxSDK.load(2, 'thread-rows').then(function (inboxSDK) {
});
inboxSDK.Lists.registerThreadRowViewHandler(function (threadRowView) {
- var threadId = threadRowView.getThreadID();
- //console.log('threadRowView', threadId, threadRowView.getThreadIDIfStable(), threadRowView.getVisibleDraftCount(), threadRowView.getVisibleMessageCount(), threadRowView.getSubject());
- console.log('got threadRowView', new Date().toString());
- if (threadRowView.getVisibleMessageCount() == 0) {
- threadRowView.addLabel(
- Kefir.fromPromise(threadRowView.getDraftID()).map((id) => ({
- title: '' + id,
- })),
- );
- }
+ threadRowView.replaceDate({ text: 'First' });
- threadRowView.addImage(
- Kefir.constant({
- imageUrl:
- 'https://lh6.googleusercontent.com/-dSK6wJEXzP8/AAAAAAAAAAI/AAAAAAAAAAA/Som6EQiIJa8/s64-c/photo.jpg',
- tooltip: 'Monkeys',
- onHover(e) {
- console.log('hovered over image', e);
- e.hoverEnd.then(() => {
- console.log('hover ended');
- });
- },
- }),
- );
-
- threadRowView.addLabel(
- Kefir.repeatedly(5000, [
- { title: 'A' },
- {
- title: 'B',
- foregroundColor: 'blue',
- iconUrl: 'https://www.streak.com/build/images/pipelineIconMask.png',
- },
- { title: 'C', foregroundColor: 'red', iconClass: 'test_icon_thing' },
- { title: 'D', iconHtml: '<div>x</div>' },
- {
- title: 'pipeline name \u00b7 stage name \u00b7 box name',
- foregroundColor: 'red',
- iconClass: 'test_icon_thing',
- maxWidth: '200px',
- },
- ]).toProperty({ title: '0' }),
- );
- threadRowView.addLabel({
- title: 'a' + i++,
- iconUrl: 'https://www.streak.com/build/images/pipelineIconMask.png',
- backgroundColor: 'white',
- foregroundColor: 'blue',
- iconBackgroundColor: 'green',
- });
-
- threadRowView.addLabel({
- title: 'pipeline name \u00b7 stage name \u00b7 box name',
- iconUrl: 'https://www.streak.com/build/images/pipelineIconMask.png',
- backgroundColor: 'white',
- foregroundColor: 'blue',
- iconBackgroundColor: 'green',
- maxWidth: '200px',
- });
-
- const iconHtml =
- "<div style='width: 56px;height: 4px;background: aqua;'></div>";
-
- threadRowView.addAttachmentIcon({
- iconHtml,
- iconClass: 'icon-html-class',
- title: 'icon html',
- });
-
- threadRowView.addAttachmentIcon(
- Kefir.repeatedly(2000, [
- {
- iconClass: 'test_icon_thing',
- title: 'thing',
- },
- {
- iconUrl: 'https://ssl.gstatic.com/ui/v1/icons/mail/gplus.png',
- title: 'blah blah',
- },
- {
- iconHtml: '<div>x</div>',
- iconClass: 'icon-html-class',
- title: 'icon html',
- },
- ]),
- );
- threadRowView.replaceDraftLabel(
- Kefir.repeatedly(1000, [
- {
- text: 'Mail Merge',
- count: 420,
- },
- {
- text: 'foo',
- },
- null,
- ]),
- );
- var r = Math.random();
- threadRowView.replaceDate(
- r > 0.33
- ? {
- text: r > 0.66 ? 'Returning in: 6 months' : 'aaa',
- tooltip: 'foo of bar',
- textColor: 'green',
- }
- : null,
- );
- threadRowView.replaceDate(null);
-
- threadRowView.replaceSubject('This is a new subect!');
-
- // threadRowView.addButton(Kefir.repeatedly(5000, [
- // {
- // iconUrl: 'https://mailfoogae.appspot.com/build/images/listIndicatorDark.png',
- // iconClass: 'buttonLight'
- // },
- // {
- // iconClass: 'test_button_thing',
- // }
- // ]));
-
- threadRowView.addButton({
- iconClass: 'test_button_thing',
- });
-
- var buttonBus = new Kefir.Bus();
- threadRowView.addButton(buttonBus.toProperty());
- buttonBus.emit({
- title: 'listIndicatorDark.png',
- iconUrl:
- 'https://mailfoogae.appspot.com/build/images/listIndicatorDark.png',
- className: 'buttonLight',
- hasDropdown: true,
- onClick: function (event) {
- event.dropdown.el.innerHTML += 'beep <b>beep</b><br>aaa<br>aaaaaa';
-
- buttonBus.plug(
- Kefir.sequentially(1000, [
- null,
- {
- iconUrl:
- 'https://mailfoogae.appspot.com/build/images/listIndicator.png',
- hasDropdown: true,
- onClick: function (event) {
- event.dropdown.el.innerHTML +=
- 'something new<p>new<p><b>more new';
-
- event.dropdown.on('unload', function () {
- console.log('thread row button dropdown closed');
- });
- setTimeout(function () {
- event.dropdown.close();
- }, 10000);
- },
- },
- ]),
- );
- },
- });
-
- threadRowView.addActionButton(
- Kefir.repeatedly(5000, [
- {
- type: inboxSDK.Lists.ActionButtonTypes.LINK,
- title: 'Take action',
- className: 'my-special-class',
- url: 'https://google.com',
- onClick(event) {
- console.log('onClick fired: ', event);
- },
- },
- {
- type: inboxSDK.Lists.ActionButtonTypes.LINK,
- title: 'Actions woo!',
- className: 'new-class',
- url: 'https://www.streak.com',
- onClick(event) {
- console.log('modified onClick fired: ', event);
- },
- },
- ]),
- );
+ setTimeout(() => threadRowView.replaceDate({ text: 'Second' }), 1000);
+ setTimeout(() => threadRowView.replaceDate({ text: 'Third' }), 2000);
});
});
In order to undo the replacement currently, you have to pass a stream (using Kefir or RxJS) and then have that stream emit null. The replaceDate and replaceDraftLabel methods act like the addLabel, addImage, addButton, etc methods in that they add an element that can be updated or removed by passing a stream and emitting values.
import * as InboxSDK from "@inboxsdk/core";
import kefirBus from 'kefir-bus';
InboxSDK.load(2, "Hello World!").then((sdk) => {
sdk.Lists.registerThreadRowViewHandler((threadRowView) => {
const dateReplacementBus = kefirBus();
threadRowView.replaceDate(Kefir.constant({ text: "First" }).merge(dateReplacementBus));
setTimeout(() => dateReplacementBus.value({ text: "Second" }), 1000);
setTimeout(() => dateReplacementBus.value({ text: "Third" }), 2000);
});
});
I do recognize that it's not terribly useful to be able to have multiple labels added by replaceDate and replaceDraftLabel, and that both of these functions are inconsistent with ThreadRowView's replaceSubject method which does act like you expect (last call overrides previous calls). We can update replaceDate and replaceDraftLabel to work as you expect.
@Macil thanks for sharing the workaround, but as you stated, the method names replaceDate and replaceDraftLabel imply that the current value is replaced 🤓 .
Moreover, I do not necessarily want to add another dependency here as we don't need to use kefir ourselves anywhere else in our extension. Would be great if we could continue using InboxSDK as an abstraction layer.
Consequently, I am looking forward to an updated version of these two methods in an upcoming release!