2
\$\begingroup\$

The Goal
Basically, what I'm looking for is the ability to click a region below an image to trigger a click() event which will scroll the window to the following image.

The Question
Is there a better way to do this? I've achieved the result I want, but with a whole ton of <divs>? I know there are some plugins out there but I'd like to do it without including a large library.

The progress...
I've done this by placing several "invisible" <div> elements around the image which are bound to click events. Later on I'll try to work through left, right, and top scrolling.

A live preview is available here.

Here is the code:

...
<div id="projimgarray">

<div class="main-proj">
    <div class="topclick"></div>
    <div class="bottomclick"></div>
    <div class="leftclick"></div>
    <div class="rightclick"></div>
    <imG class="main-proj-img" src="https://picsum.photos/800/600">
</div>
                        
<div class="main-proj">
    <div class="topclick"></div>  // these are the "invisible divs"
    <div class="bottomclick"></div>
    <div class="leftclick"></div>
    <div class="rightclick"></div>
    <img class="main-proj-img" src="https://picsum.photos/801/600">
</div>
$("document").ready(function() {

    $(".bottomclick").click(function() {
        console.log("Scroll jquery is working");
        $( this ).parent().addClass("activeproj");
        let $target = $(".activeproj").next(".main-proj");
        if ($target.length == 0 ) 
            $target = $(".main-proj:first"); 


        $([document.documentElement, document.body]).animate({
        scrollTop: $($target).offset().top
        }, 2000);

        $('.activeproj').removeClass('activeproj');
    });
});
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

I think the overall approach of having lots of <div>s to act as clickable areas is just fine. An alternative is to dynamically calculate the position of a click on main-proj and see if it's sufficiently in one of the directions to count as an intent to scroll, but that's more complicated and more prone to breakage than having separate divs.

Don't be afraid of WET HTML markup in the rendered output. It's perfectly normal and isn't inelegant or indicative of a problem. It's true that having huge numbers of DOM nodes on a page can slow things down, but 4 extra <div>s for each image isn't incredibly likely to be a tipping point for that - and if it does turn out to be one, consider changing the page so that only a reasonable number of .main-projs are rendered at a time, like 50 or 100, but not much more than that.

That said, it would be good to avoid WET HTML in your source code if possible, if feasible given your setup - you don't want to copy-paste those 7 lines of

<div class="main-proj">
    <div class="topclick"></div>
    ...

over and over again every time you want to add an image. It'll make understanding the code at a glance harder, and can make refactoring more difficult. To only write the template for such a section once, you can either do it server-side with a template engine, or client-side with something like React, or even with vanilla DOM methods like document.createElement.

Another thing to consider - if the different clickable divs result in different functionality depending on which was clicked, that would be a great thing to indicate to the user. For example, you might want to change the color of a hovered div, perhaps with an arrow or something, to give the user a visual indicator of what will happen when clicked. This is trivial to achieve with separate <div>s, but annoyingly complicated without it. (If you don't give the user a visual indicator of the difference between clicks on different sections of .main-proj, it's probably not as user-friendly as it should be.)

Other suggestions regarding your code:

Use const, avoid let - see here. It's good to indicate to future readers of the code (often including yourself) that a variable will not be reassigned.

Don't re-select elements you already have a reference to - you don't need to select the .activeproj again, select the parent again instead, or save it in a variable:

const $thisProj = $(this).parent();
// do you want to remove .activeproj from other elements at this point?
// or, if activeproj isn't used in the CSS, you could remove it completely
$thisProj.addClass('.activeproj');
const $target = $thisProj.next('.main-proj');
// ...

Unless you have other kinds of children in this projimgarray (I think you shouldn't), you can also omit the parameter to .next.

No need to wrap a jQuery object in jQuery again This:

scrollTop: $($target).offset().top

can be

scrollTop: $target.offset().top

projimgarray? Arrays are a JavaScript construct. DOM elmements aren't really arrays. Perhaps call it something more precise, and use dashed-case per convention (and to make the word separations easier to understand at a glance) - maybe something like all-projects.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.