Fix missing fragment events in scrollview #3580
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ent-events-in-scrollview
Closed
|
Good catch. This should definitely be addressed. We already have logic for dispatching these events in the fragments controller. Instead of duplicating that code in the scroll view I think we should always trigger the events from one place (the fragments controller). https://github.com/hakimel/reveal.js/blob/master/js/controllers/fragments.js#L312-L332 It'll take a little refactoring but shouldn't be major. Will take a stab at it. |
hakimel
added a commit
that referenced
this pull request
Feb 26, 2024
|
Was able to fix this by relocating the fragment event dispatchers so no new code was needed. Added tests too. Thanks again for discovering and highlighting the issue! |
|
Even better! The fragment controller is where I got the code from. While I don't fully understand why event dispatching has to be confined to event target's controller, I do understand that changing the placement of code is better than having duplicate code in multiple locations across multiple files. I thought about messing with the fragment controller but I was worried I might break things without knowing (or without knowing the issues to test for). Anyways, I appreciate that you took time to find a better solution! |
lenis0012
added a commit
to chilit-nl/reveal.js
that referenced
this pull request
Jun 13, 2024
* Support multiple aside notes elements per slide So far, multiple notes per slide are only supported if they are attached to fragments (without fragments, only the first aside notes element on a slide is displayed). With this commit, the contents of all notes on a slide are displayed (except for fragments with notes, for which, as before, only each fragment's first/single note is displayed). * Add generated files * update browserlist from '> 0.5%, IE 11, not dead' to '> 2%, not dead' hakimel#2985 * Missing lang attribute The lang attribute does not default to English. It defaults to an unknown, which is an accessibility issue. * fix issue with past/future vertical slides remaining visible in Safari 15.4 (closes hakimel#3164) * fix issue were auto-animate could interfere with inherited line-height * Update js.yml Signed-off-by: sashashura <[email protected]> * prevent extra \n at end of single notes hakimel#3010 * fix merge error, closes hakimel#3277 * remove doppler * fix getAttribute exception in notes plugin hakimel#3285 * 4.4.0 * Gulp livereload: include subfolders to watch for changes in html and md Currently it's only watching for changes to `.html` and `.md` files located in the root of the project. I was frustrated when livereload stopped working for me: turns out it was because I put my content into subfolders. * roll back unintended change to index content * Add theme 'dracula' New theme using color palette from [Dracula](https://draculatheme.com/) dark editor theme available for quite a number of different applications. * 2023 * only test one node version * run tests in node 14 * audit fix * spec updates * adds jump-to-slide, press G to activate * add jump-to-slide to help overlay, style tweaks * delay slide jumps a few ms * update hljs 10 > 11.7, fix perf issue in demo presentation caused by auto lang detection * jump-to-slide is 1-indexed, falls back on word search * better selection color contrast for black theme * enforce a min length on jump to slide search queries * fix incorrect condition for jump-to-slide * jump to slide tweak * fix security alerts by upgrading glob-parent hakimel#3343 * scope print styles to .reveal hakimel#3348 * Fixes typos * Correctly strip leading white-space from markdown If the markdown contains something that is indented by more that the `leadingTabs`/`leadingWs` then extra white space is incorrectly removed. ie the following example: ``` <section data-markdown> some text indented text more indented text </section> ``` would result in the following markdown: ``` some text indented text more indented text ``` We can work around this problem by using a function to generate the replace value. * Add RFC3986-compliant URL format encoding Fixes hakimel#3315 * add sortFragmentsOnSync option, makes it possible to avoid unwanted sorting in editing environments like slides.com * Fix overview spacing for disabled auto layout (hakimel#3291) * Fix overlap in overview when config.disableLayout === true * run gulp js after commit 9193e5c --------- Co-authored-by: Hakim El Hattab <[email protected]> * remove commented out config * Black & White compact themes with verbatim headers. (hakimel#3310) * rename high contrast themes, dont change anything else compared to black/white themes hakimel#3310 * reduce fragment style specificity, add custom class to reset fragment styles hakimel#2927 * build latest css * 4.5.0 * add ln-start-from for code sections to markdown * add tests * fix code block auto-animate bug that caused unmatched lines to appear without fading in * auto-slide duration falls back on global setting instead of looking at first fragment * fix issue where fragment-evel autoslide timing was when multiple fragments share the same index * fixes livereload when using root CLI param * ignore node_modules for livereload may allow perf boost * allow theme subfolders allows custom themes to import files from subfolders inside the `css/theme/source` folder. in `css/theme/source/custom-theme.scss` we can now do ```scss @import `custom-theme/controls` @import `custom-theme/headings` ... ``` * adds ability to override markdown default options ```js Reveal.initialize({ markdown: { defaultOptions: { verticalSeparator: '\n--\n`, } } }) * add support for links to the id of an element nested inside slide fixes hakimel#3231 * build * move markdown default options to top level hakimel#3443 * update markdown default notes separator to ignore inline occurances of 'notes:', closes hakimel#1915, closes hakimel#2762 * fix scss watch tasks broken on syntax error when there is a syntax error in a sass file (theme or core) the npm start command used to hang, forcing the user to stop and restart the task to compile again. this fix allows to keep the start-task watching/compiling even when there is an error : - the error is displayed in terminal - the rest of gulp tasks are not called (no reload in the browser) - the user can edit the scss files to try a fix without the need to stop/restart the `npm start` command * speed up livereload connect.reload needs a stream of files to fire, but this stream is irrelevant here and slows refresh time a lot (from ~2ms to 2000ms here) * Fix dracula's theme list-style on sub-items * fix typos in variable names * Refactored var to let or const, strict equality * build md plugin hakimel#3454 * Update demo.html switch themes keep current slide * fix code blocks font when printing pdf fixes hakimel#2867 * add start/stopEmbeddedMedia API methods for controlling playback of video/audio/iframes * fix dracula li markers replace :before pseudo elements with :marker selector allows to have different marker based on the level of nesting (as in other themes : disc, square, circle) * fix dracula theme li numbering * refactor dracula theme sass code * add test deck with 500 slides * 4.6.0 * fix alpha overlap during scrolled code highlight transitions * foundation for reader mode, activate via 'mode=reader/print' config param * 4.6.1, remove log * separate reader mode into individual controller, add scroll triggers for fragments * revert demo changes to index.html * reader mode; deeplink support, presentation scaling, scroll trigger fixes * refactoring, fix preload distance * fix empty slide bug when all slides in a stack are hidden via data-visibility * Notes plugin: notes from data-notes attribute were not shown * add support for aside element notes inside of fragments (fixes hakimel#3478) * reader mode can be turned off without reload, add Reveal.toggleReader() * revamped reader mode sticky logic, add option for fullscreen pages * rename 'mode' config value to 'view' * reader mode supports scroll snapping, sticky pages with scroll triggers are always full height * fix preload bug * refactoring, remove unused layout * dispatch slidechange events in reader mode * fix incorrect unit for slide-width/height css variable hakimel#1263 * fix issues with active slide logic in reader mode, foundational work for auto-animate support * update api method name * Support to 'wheel' event listener * add support for responsively activating reader mode via * reader mode tests * refactoring * reader mode; named deeplink support, stay on same slide when reader mode is turned on/off * reader mode progress bar * reader mode progress bar can be dragged to scroll * finishing touches on reader mode progress bar * refactoring * mobile tweaks * reader progress theming, automatically invert based on slide bg * convert sass controls spacing to css var, full height reader progress bar * disable overview while in reader mode * audit fix * more accurate scroll trigger positioning in progress bar * reader mode tweaks * reader mode accessibility, bug fixes * readerScrollBar -> readerScrollbar * rebuild assets * prevent extra page at end when printing to pdf, reader mode styling tweaks * don't show reader scroll bar when there is no overflow, reader style tweaks * === * audit fix * add scroll snap points for reader mode scroll triggers * massive reader mode refactor; adds support for auto-animate + snapping for fragments * major cleanup of reader mode code * improved reader progress bar visuals in high density * reader mode refactoring * reader mode remembers scroll position when reloading * fix slide numbers not visible in pdf exports * ? keyboard shortcut should not trigger when focus is on an editable element fixes hakimel#2645 * reader mode now works for embedded decks * fix scroll snapping in reader mode compact layout * reader mode -> scroll view, auto-enable below 435px width * fix scroll view activation in tests * not so important * not important * remove legacy mousewheel listeners hakimel#3489 * 5.0 * mute video in scroll demo * chore: fix typos * chore: remove deprecated css declarations * chore: bump deps * rebuild after deps update * scroll example deck tweaks * fix speaker view bug, bump version to 5.0.1 hakimel#3512 * md plugin api works even if deck isn't available hakimel#3517 * nil check for deck in md plugin hakimel#3517 * fix issue where background of a future vertical slide is briefly visible ahead of time hakimel#3520 * 5.0.2 * jump-to-slide; add support for 'h.v' format, adapat to match slide number format hakimel#3501 * fix exception when stepping backwards through code highlights hakimel#3524 * bump 5.0.2 version in build files * search plugin; allow searching for any character (was alphanum) hakimel#2331 hakimel#3532 * search plugin; search for whole phrase hakimel#2331 hakimel#3532 * fix notes in pdf print view hakimel#3535 * fix exception when navigating decks on mobile browsers hakimel#3539 * fix broken mobile scroll view navigation where there were fragments starting at an index above 1 hakimel#3540 * fix pause/help overlay position in scroll mode (closes hakimel#3542) * fix: use `setAttribute` instead of `innerHTML` to prevent xss * rebuild * fix missing backgrounds when scroll view is actived responsively (fixes hakimel#3554) * 2024 * add support for keyboard navigation in scroll view hakimel#3515 * fix xss issue reported by @realansgar, regression from 3dade61 * add logs to scrollview.js * update build * update build * fix slide backgrounds being replaced by global background * fix selector constant * remove console logs * fix selector constant * add comment for background fix * tweaks for hakimel#3568 * add F1 key to toggleHelp for non-english keyyboards * indentation tweak * fix fragment events not firing in scroll view + add tests hakimel#3580 * 5.0.5 * new .enter-fullscreen class lets you add shortcuts to fullscreen mode * fix broken backwards navigation in rtl mode * dont prevent swipe navigation on video backgrounds hakimel#3584 * allow same background video to continue playing across multiple slides hakimel#3189 hakimel#2882 Co-authored-by: Chi Vong <[email protected]> * don't restart media when it's already playing hakimel#2882 * fix issue when disabling autoPlay config flag at runtime * don't start video bgs if autoPlayMedia config is set to false * fix previous bg video playing in background * fix rtl prev/next navigation on slides with fragments * Notes: don't error on non-string message * fix vertical swipe navigation not blocking page scrolling in embedded decks * fix exception when destroying uninitialized reveal instance (closes hakimel#3593) * MathJax3: allow non-singleton Reveal instance * fix r-stack with `grid-template-rows: 100%;` * update build * nil check slides before running auto-animate transition hakimel#3592 * auto-animate demo tweak * Re-add question mark for help * 5.1.0 * chore: update package lock * feat: add first draft of jspring 2024 keynote slides * chore: updates slides * chore: update slides with new graphics * feat: add breakout alides * feat: add extra slides based on feedback --------- Signed-off-by: sashashura <[email protected]> Co-authored-by: Jens Lechtenbörger <[email protected]> Co-authored-by: hakimel <[email protected]> Co-authored-by: Sam <[email protected]> Co-authored-by: Alex <[email protected]> Co-authored-by: Andrey Mikhaylov (lolmaus) <[email protected]> Co-authored-by: Juhamatti Niemelä <[email protected]> Co-authored-by: Andreas Deininger <[email protected]> Co-authored-by: John Kristensen <[email protected]> Co-authored-by: Martino <[email protected]> Co-authored-by: Elliot" Constantin H <[email protected]> Co-authored-by: Peter Kehl <[email protected]> Co-authored-by: Florian Klien <[email protected]> Co-authored-by: t-fritsch <[email protected]> Co-authored-by: Gildasio Junior <[email protected]> Co-authored-by: Artur Neumann <[email protected]> Co-authored-by: Prarup Gurung <[email protected]> Co-authored-by: Ricardo Menotti <[email protected]> Co-authored-by: Yevhen Kozlov <[email protected]> Co-authored-by: NGUYEN DINH Quoc-Huy <[email protected]> Co-authored-by: Mr.Hope <[email protected]> Co-authored-by: Michael Wang <[email protected]> Co-authored-by: Anas Bouzid <[email protected]> Co-authored-by: Christian Ziemski <[email protected]> Co-authored-by: Chi Vong <[email protected]> Co-authored-by: Nat Karmios <[email protected]> Co-authored-by: Wang Guan <[email protected]> Co-authored-by: alifeee <[email protected]>
srwohl
pushed a commit
to srwohl/phantom-pres
that referenced
this pull request
Sep 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fragmentshownandfragmenthiddenevents are not triggered when fragments appear and dissappear in scrollview. The code added is just the code that is usually executed that results in the dispatch of these events. This code was carefully placed so that these events are not triggered when they shouldnt be like when the slide comes in and the default fragment automatically appears with the slide. The latter is treated like more of a slide change rather than a fragment being shown.