Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Journal "hover" highlight effect only works after an Edit #16

Closed
StevenBlack opened this Issue Aug 20, 2011 · 9 comments

Comments

Projects
None yet
3 participants
Contributor

StevenBlack commented Aug 20, 2011

Problem: Journal "hover" highlight effect only works after an Edit

Reproduce: Load any page with div.journal>span items. Hover over the div.journal>span items.

Observe: No story highlighting (yellow background) is displayed around the appropriate element.

Expected: Story highlighting (yellow background) should be evident.

Continue: Edit a paragraph. Save changes. Hover over the div.journal>span items.

Observe: Story highlighting (yellow background) is evident.

@StevenBlack StevenBlack added a commit to StevenBlack/Smallest-Federated-Wiki that referenced this issue Aug 20, 2011

@StevenBlack StevenBlack Resolves issue #16. Uses $(".journal").delegate(".action", "hover", .…
….. ) rather than attaching mouseover() and mouseout() events to each div.journal>span.action.

This has the benefit of "wiring" the events before any edits so hover effects work out-of-the-box. This also automatically wires subsequently added ".action" DOM elements.

IMPORTANT: Assumes that Pull Request #15 has landed because this uses class ".action", not class ".edit" that was previously used.

See Pull Request #15, especially the discussion and the amended files therein.  WardCunningham#15

Signed-off-by: Steven Black <steveb@stevenblack.com>
518b4ba
Owner

WardCunningham commented Aug 20, 2011

To reproduce this bug one must look at the server-side rendering of a page. The client-side rendering (with view) works ok. Compare:

http://127.0.0.1:4567/indie-web-camp
http://127.0.0.1:4567/view/indie-web-camp

It has been a bit of a struggle to keep these two versions in sync.

Owner

WardCunningham commented Aug 26, 2011

I'm still wanting to employ delegate() but have to reorganize the code a bit to make it work when internal links add new pages client-side.

Contributor

StevenBlack commented Aug 26, 2011

All events within a delegate container -- be they from new links or not -- bubble to the container and will be processed.

Therefore this is a code-ectomy, no server-side code required at all. This is as it should be, localizing client-side hover highlighting in the client.

Other upside: one $.delegate() call will handle hover events on potentially hundreds of action links, including those actions added later.

It's all-good.

Owner

WardCunningham commented Aug 26, 2011

$(".journal").delegate(".action", "hover", ...

I hate to be dense. And I could just try this, but ...

It seems to me that this creates delegates for all the .journals in existence when the statement runs. If more .journals are created then delegate() will have to run again to pick them up. This could happen when sortable gets set up as it is a similar workflow.

This is made more confusing in that the "view" url routing is just a work-around for code that hadn't been written when you offered this commit.

Contributor

StevenBlack commented Aug 28, 2011

Ah, I wasn't aware this was a requirement.

Events bubble-up through the DOM so, at the limit, we could use $("body").delegate( ".action", "hover", .... )

Of course this won't be as efficient as selecting a more specific parent container but that's our catch-all.

If journals are smart, as I suspect they eventually will, then we could make them wire themselves, with something like $( this ).delegate( ".action", "hover", .... ) upon creation, or have a potential journal factory do it.

Owner

WardCunningham commented Aug 28, 2011

I like the $("body") idea. I'm not opposed to having the dom work hard for us. In fact, since this is a global operation, that seems to be the right place to delegate from. If you revise it and check out all these cases, I'll pull it gladly. I'm also happy to do it too.

Owner

WardCunningham commented Aug 29, 2011

resolved by be2e970

FND commented Aug 29, 2011

I like the $("body") idea

FWIW, I believe $(document.body) is a little faster (a quick comparison confirms this - though it's barely significant).

resolved by be2e970

That commit is currently 404; did you perhaps forget to push?

Owner

WardCunningham commented Aug 29, 2011

I attached the delegation to .main, the div around the pages.

(I thought it interesting that I could refer to a commit that I hadn't yet pushed. This simplified my workflow. However, there are drawbacks I hadn't considered. Might be time for a two-phase commit?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment