Skip to content

fix(deck-options): Easy Days accidentally set [HACK]#20935

Open
david-allison wants to merge 1 commit intoankidroid:mainfrom
david-allison:20929-easy-days
Open

fix(deck-options): Easy Days accidentally set [HACK]#20935
david-allison wants to merge 1 commit intoankidroid:mainfrom
david-allison:20929-easy-days

Conversation

@david-allison
Copy link
Copy Markdown
Member

Note

Assisted-by: Claude Opus 4.7 - implementation

Purpose / Description

On a vertical scroll, easy days values were set.

Fixes

Approach

This is fixed via a hacky JS wrapper as touch-action:pan-y had no effect

Due to the size & maintenance burden of a 'proper' fix [as this code should be temporary], I selected a small, readable piece of JS which disables all touch events, and re-implements 'click'.

The 'drag-to-set' functionality of the range slider is lost, but since Easy Days is only three distinct values, and this should be a temporary fix, this feels acceptable

Rejected approach
Here is the 'more correct' code which I rejected. Claude Opus 4.7. A request ot make it less verbose kept it at ~50 LOC and made it unreadable.

       val js =
            """
            (function() {
                if (document.__ankidroidEasyDaysGuard) return;
                document.__ankidroidEasyDaysGuard = true;
                const SLOP = 10;
                function wrap(slider) {
                    if (slider.__ankidroidWrapped) return;
                    slider.__ankidroidWrapped = true;
                    const container = slider.closest('.input-container') || slider.parentElement;
                    if (!container) return;
                    slider.style.pointerEvents = 'none';
                    container.style.touchAction = 'pan-y';
                    const setFromX = (x) => {
                        const rect = slider.getBoundingClientRect();
                        if (rect.width <= 0) return;
                        const ratio = Math.max(0, Math.min(1, (x - rect.left) / rect.width));
                        const min = parseFloat(slider.min) || 0;
                        const max = parseFloat(slider.max) || 100;
                        const step = parseFloat(slider.step) || 1;
                        const value = Math.round((min + (max - min) * ratio) / step) * step;
                        if (parseFloat(slider.value) !== value) {
                            slider.value = value;
                            slider.dispatchEvent(new Event('input', { bubbles: true }));
                            slider.dispatchEvent(new Event('change', { bubbles: true }));
                        }
                    };
                    let active = false, startX = 0, startY = 0, decided = false, horizontal = false;
                    container.addEventListener('touchstart', (e) => {
                        if (e.touches.length !== 1) return;
                        const t = e.touches[0];
                        const rect = slider.getBoundingClientRect();
                        if (t.clientX < rect.left || t.clientX > rect.right ||
                            t.clientY < rect.top || t.clientY > rect.bottom) return;
                        active = true;
                        decided = false;
                        horizontal = false;
                        startX = t.clientX;
                        startY = t.clientY;
                    }, { passive: true });
                    container.addEventListener('touchmove', (e) => {
                        if (!active || e.touches.length !== 1) return;
                        const t = e.touches[0];
                        if (!decided) {
                            const dx = Math.abs(t.clientX - startX);
                            const dy = Math.abs(t.clientY - startY);
                            if (dx + dy < SLOP) return;
                            decided = true;
                            horizontal = dx > dy;
                            if (!horizontal) { active = false; return; }
                        }
                        e.preventDefault();
                        setFromX(t.clientX);
                    }, { passive: false });
                    container.addEventListener('touchend', (e) => {
                        if (!active) return;
                        active = false;
                        if (!decided && e.changedTouches[0]) setFromX(e.changedTouches[0].clientX);
                    }, { passive: true });
                    container.addEventListener('touchcancel', () => { active = false; }, { passive: true });
                }
                const setupAll = () => document
                    .querySelectorAll('.easy-days-settings input[type="range"]')
                    .forEach(wrap);
                setupAll();
                new MutationObserver(setupAll).observe(document.body, { childList: true, subtree: true });
            })();
            """.trimIndent()

How Has This Been Tested?

Google Pixel 9 Pro

  • Scrolling vertically no longer sets the values ✅
  • Tapping sets the values ✅
  • Drag to set no longer works ⚠️

Checklist

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

On a vertical scroll, easy days values were set.

This is fixed via a hacky JS wrapper as touch-action:pan-y had no effect

Due to the size & maintenance burden of a 'proper' fix [as this code
should be temporary], I selected a small, readable piece of JS which
disables all touch events, and re-implements 'click'.

The 'drag-to-set' functionality of the range slider is lost, but since
Easy Days is only three distinct values, and this should be a temporary
fix, this feels acceptable

Fixes 20929

Assisted-by: Claude Opus 4.7 - implementation
@ZornHadNoChoice

This comment was marked as resolved.

@david-allison
Copy link
Copy Markdown
Member Author

Since this removes dragging, why not change them to radio buttons like they were before https://forums.ankiweb.net/t/easy-days-ui-improvement-different-button-size/52665/27 ?

We get this screen from Anki. I've pinged the author about a proper fix.

Here, I wanted something maintainable, understandable & quick to fix the bug on our side, [as this bug has resulted in a major scheduling issue].

For a longer-term fix, it's better to have it fixed at the source in the Anki code, rather than us diverging from Anki Desktop, and having to maintain a separate implementation

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deck options Easy Days switch triggered when scrolling

2 participants