Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #285, migrate to TypeScript #334

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

stil
Copy link

@stil stil commented Sep 25, 2024

This PR migrates the code to TypeScript.

I kept Babel for compilation to keep it small.

There were a couple of type mismatches that I believe are bugs:

Type declarations coming from focus-lock need fixing too (focusSolver can accept arrays):

diff --git a/node_modules/focus-lock/dist/es5/moveFocusInside.d.ts b/node_modules/focus-lock/dist/es5/moveFocusInside.d.ts
index 2f28995..b173bfe 100644
--- a/node_modules/focus-lock/dist/es5/moveFocusInside.d.ts
+++ b/node_modules/focus-lock/dist/es5/moveFocusInside.d.ts
@@ -13,5 +13,5 @@ interface FocusLockFocusOptions {
  * HTML markers (see {@link import('./constants').FOCUS_AUTO} constants) can control autofocus
  * @see {@link focusSolver} for the same functionality without autofocus
  */
-export declare const moveFocusInside: (topNode: HTMLElement, lastNode: Element, options?: FocusLockFocusOptions) => void;
+export declare const moveFocusInside: (topNode: HTMLElement | HTMLElement[], lastNode: Element | null, options?: FocusLockFocusOptions) => void;
 export {};
diff --git a/node_modules/focus-lock/dist/es5/sibling.d.ts b/node_modules/focus-lock/dist/es5/sibling.d.ts
index d318b13..4a02d0f 100644
--- a/node_modules/focus-lock/dist/es5/sibling.d.ts
+++ b/node_modules/focus-lock/dist/es5/sibling.d.ts
@@ -42,13 +42,13 @@ interface FocusNextOptions {
  * @param fromElement - common parent to scope active element search or tab cycle order
  * @param {FocusNextOptions} [options] - focus options
  */
-export declare const focusNextElement: (fromElement: Element, options?: FocusNextOptions) => void;
+export declare const focusNextElement: (fromElement: Element | null, options?: FocusNextOptions) => void;
 /**
  * focuses prev element in the tab order
  * @param fromElement - common parent to scope active element search or tab cycle order
  * @param {FocusNextOptions} [options] - focus options
  */
-export declare const focusPrevElement: (fromElement: Element, options?: FocusNextOptions) => void;
+export declare const focusPrevElement: (fromElement: Element | null, options?: FocusNextOptions) => void;
 declare type FocusBoundaryOptions = Pick<FocusNextOptions, 'focusOptions' | 'onlyTabbable'>;
 /**
  * focuses first element in the tab-order

Anyway, in the current state it requires some minor polishing but I leave it to you to verify.
Hopefully it saves you some work and could speed up introducing TS.

@theKashey theKashey self-requested a review October 11, 2024 08:58
@theKashey
Copy link
Owner

Thanks, this definitely can save me heaps of time

@theKashey theKashey mentioned this pull request Oct 23, 2024
Copy link

stale bot commented Dec 10, 2024

This issue has been marked as "stale" because there has been no activity for 2 months. If you have any new information or would like to continue the discussion, please feel free to do so. If this issue got buried among other tasks, maybe this message will reignite the conversation. Otherwise, this issue will be closed in 7 days. Thank you for your contributions so far.

@stale stale bot added the state label Dec 10, 2024
@stil
Copy link
Author

stil commented Dec 10, 2024

This is still relevant.

@stale stale bot removed the state label Dec 10, 2024
@theKashey
Copy link
Owner

Please dont worry about conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants