Thanks for all of the info, it really helps a lot. I am probably going to build this up from a prototype less than ideal state and then layer on better design principles as incremental fixes. I have a feeling that if I try to get the 100% solution in the first shot, it will be 2 weeks before I make a commit and the code review will be horrifically complicated.
~Michael On Nov 23, 6:09 pm, David Hearnden <[email protected]> wrote: > On Wed, Nov 24, 2010 at 11:30 AM, Michael MacFadden > > <[email protected]> wrote: > > All, > > > I am about halfway through implementing the new style inline thread > > indicators showing the count of read/unread blips. I have faked the > > current count numbers to this point, focusing mainly on the rendering > > issues. I am ready to takle the count and read / unread state > > changes. I had a couple questions I was hoping the community might be > > able to help with: > > > 1) > > > What are the rules for total blip count? I would assume we only want > > to count the blips that will be visible. There seems to be blips in > > the conversation thread that might not be visible. > > There is a bit of history here. In the conversation model, some blips > are marked as deleted (blip.isDeleted()). This situation arises if > someone has deleted a blip, but there are still replies to it that > have not been deleted. This is one of those arbitrary divergences > between inline and non-inline replies - when you delete a blip, only > its inline replies get deleted with it too; non-inline replies, > because they are rendered outside the focus frame, do not get deleted. > These deleted-but-still-living-in-the-model blips, called > 'tombstoned' blips, are not rendered in Google Wave, which is the only > case where blip in the model are not presented in the wave panel. > > However, for WIAB, we've abandoned that model, for a few reasons: > a) all replies are now rendered within the focus frame (either > anchored or unanchored), so all replies should be deleted when a blip > gets deleted, so these tombstoned blips should never occur anyway. > b) choosing not to render tombstoned blips just makes the UI even more > confusing, because the context of the free-floating reply threads is > ambiguous. > > So, in conclusion, I think the result is that there are no longer any > blips that should be suppressed from presentation, so using the model > to count blips should be sufficient. In addition, we want to reserve > the property that although all blips in the model should be presented, > we only want to work with the constraint that they will eventually be > rendered, not that they are rendered at any one time. This is > important for paging content based on where the viewport is, and for > incremental background rendering of wave content, and for potentially > skipping rendering collapsed threads until they get expanded, etc. > For that reason, the view structure should not be used for blip > counting, which makes counting using the model necessary. > > > > > 2) > > > Right now I can only ask the conversation thread for an iterator of > > the blips. There doesn't seem to be a convenient way to get the count > > of blips, short of calling getBlips() and iterating through them. > > This could be added. > > That's right. However, since each blip needs to be queried for its > read/unread status, the presentation logic is doing work per blip > anyway, and can produce total counts as it goes. But for an initial > prototype (e.g., just displaying total counts), you can use > Iterables.size(thread.getBlips()) to save typing. Yes it's O(n) > rather than O(1), but the full counting implementation is probably > going to be O(n) anyway. > > > 3) > > > I assume I can use the SupplementImpl class to see if a Blip is read > > or not during rendering. > > The SupplementImpl is indeed the logic that determines read state. > However, there is already an instance floating around: > StageTwo.getSupplement(), which exposes ReadableSupplementedWave. > That interface has an isUnread(ConversationBlip) method you can use to > query for read state. > > The brief summary of the supplement layers are as follows: > > 1. PrimitiveSupplement [implemented by > PrimitiveSupplementImpl/WaveletBasedSupplement ] > A data structure in which per-user state is stored. This API just > exposes the data, no interpretation is embodied here. > > 2. {Readable/Writable}Supplement [implemented by SupplementImpl] > Adds interpretation to the PrimitiveSupplement, exposing semantic > actions in terms of wavelet state (e.g., isBlipUnread(waveletId, > blipId, lastModifiedVersion)). > > 3. {Readable/Writable}SupplementedWave [implemented by > SupplementedWaveImpl & LiveSupplementedWaveImpl] > Curries/binds a {Readable/Supplement} with a particular Wave, to > expose supplement actions in the context of a wave (e.g., markAsRead() > no parameters). > > The layers could probably be collapsed a bit now. Historically, there > were many components in Google Wave that needed to use supplements, > but wanted to hook into (or make copies of) different levels of state > for efficiency reasons, rather than everything using the same > top-level SupplementedWave thing (which requires a full wave model > underneath). > > > 4) > > > I know that I COULD respond to the edit actions like continuation and > > delete to trigger and update of the count, however this would be the > > wrong approach because it wouldn't handle when some one else is > > adding / removing blips. To this point I haven't seen how to observe > > state changes in the conversation model and trigger rendering events. > > I.e. how do I notice another user has added a blip in some other > > browser so I can trigger calling my Dom Impl class to update the count > > in my browser. > > > I just haven't been introduced to this event mechanism yet. > > In general, we try not to do change the UI in response to UI actions, > but instead let the UI actions take place on model objects, and those > model objects broadcast changes, which are then picked up by renderers > that update the UI. This keeps a single code path for changing the > UI, for both local and remote actions. > > The reading control logic is implemented in > wavepanel.impl.reader.Reader, which performs supplement actions in > response to UI focus-frame movements. Moving the focus frame around > causes the Reader to mark those blips as read. > Similarly, the reply control logic is implemented in > wavepanel.impl.edit.Actions, which performs conversation actions in > response to UI edit actions (reply,delete etc). > > The models that are updated by the reading and edit control logic (the > Conversation and Supplement models) are both observable, and broadcast > events on changes like blip addition/deletion and blip read/unread. > The liveness of the UI is provided by LiveConversationViewRenderer, > which attaches itself as an observer to the conversation and > supplement models. Blip addition/removal events trigger incremental > rendering, and supplement read/unread events trigger re-rendering the > read/unread state of blips. Since the thread counts are part of the > UI, they are part of the rendering, so that logic should probably hook > in to the LiveConversationViewRenderer somehow. The counting logic > may become complex/stateful enough to warrant pushing all the > supplement-related rendering concerns to a separate class. > > The first thing you'll notice with the supplement events is that they > are not as useful as they could be. For robustness, they > pessimistically broadcast maybe-read-status-changed events, rather > than exact read-status-changed events. i.e., if a blip changes > between read and unread, the supplement model guarantees it will tell > you that it might have changed, but it may also tell you that it might > have changed when the read/unread state hasn't changed at all. There > is no reason why the event model can't be exact (in fact, the code is > probably already broadcasting events precisely when exact changes > occur, so it may be trivial to fix this up); it's pessimism is just a > piece of legacy. The relevance for the UI thread counts, however, is > that since the event API does not tell you what has changed, you'll > need to maintain the exact read/unread state of every blip yourself, > in order to determine when read->unread or unread->read transitions > occur (as opposed to false positive read->read or unread->unread > maybe-events). If you know exact transitions, then the update logic > can be expressed as increments/decrements to the containing threads' > counters, rather than full recounts on any change. If you don't know > exact transitions, then you need something like maintaining full > read/unread blip collections for each thread... yuk. A corrective > layer to turn maybe-events into exact events is probbaly going to be > essential because of Undercurrent's stateless view-object approach, > where all presentation state should be in the DOM (so that the client > can respond to UI events on server--rendered HTML with minimal startup > overhead, like rebuilding a full read/unread collection for every > thread in the entire wave). Having something like: > > <div kind='anchor' read='23' unread='3'> > ... > <span class='bubbleText'> 23 (3) </span> > </div> > > would be feasible for storing the read/unread counts as state in the > DOM. Storing something like: > <div kind='anchor' > readBlips='[b+9sndf8,b+oamx9s,b+5c2sd8x,b+opisdkf,...]' > unreadBlips='[b+skdf980s,b+kflhojp,...]'> > ... > </div> > > seems a bit insane. In Google Wave, there is some code to maintain > the exact read/unread state of each blip, compensating for the inexact > event API of the supplement model. However, IIRC, that code does not > build up the read/unread information incrementally; it does a full > pass over the entire wave before you can use it. I'll dig that out > and send it to you for inspiration, or maybe I can find somewhere to > patch it in directly, but ideally you can find a solution where the > client needs minimal upfront processing before being able to update > the UI counters in response to a read/unread change of some blip > somewhere. I don't know whether that's possible or not though, I > haven't convinced myself either way. > > Sorry to bombard all this information at you, but I hope it gives > clear answers to your excellent questions. > > -Dave -- You received this message because you are subscribed to the Google Groups "Wave Protocol" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/wave-protocol?hl=en.
