-
Notifications
You must be signed in to change notification settings - Fork 29
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
feat: allow third click to disable sorting (fixes #132) #139
Changes from 4 commits
b2c910c
1a597ea
0b21452
845c7b3
f4b9066
471021b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,8 @@ export class SortingManager { | |
private _filterTextBox: HTMLInputElement; | ||
private _sortingButtons: HTMLElement; | ||
private _instanceId: string; | ||
private _sortingState: { [key: string]: "unsorted" | "ascending" | "descending" } = {}; // Track state for each sorting option | ||
private _filterTimeout: number | null = null; // Timeout ID for debouncing | ||
|
||
constructor(filtersId: string, sortingOptions: readonly string[], instanceId: string) { | ||
const filters = document.getElementById(filtersId); | ||
|
@@ -17,6 +19,11 @@ export class SortingManager { | |
this._instanceId = instanceId; | ||
this._filterTextBox = this._generateFilterTextBox(); | ||
this._sortingButtons = this._generateSortingButtons(sortingOptions); | ||
|
||
// Initialize sorting states to 'unsorted' for all options | ||
sortingOptions.forEach((option) => { | ||
this._sortingState[option] = "unsorted"; | ||
}); | ||
} | ||
|
||
public render() { | ||
|
@@ -65,27 +72,26 @@ export class SortingManager { | |
} | ||
}; | ||
|
||
// Observer to detect when children are added to the issues container | ||
// Observer to detect when children are added to the issues container (only once) | ||
const observer = new MutationObserver(() => { | ||
if (issuesContainer.children.length > 0) { | ||
observer.disconnect(); // Stop observing once children are present | ||
if (searchQuery) filterIssues(); // Filter on load if search query exists | ||
} | ||
}); | ||
|
||
// Start observing the issues container for child elements | ||
observer.observe(issuesContainer, { childList: true }); | ||
|
||
// Add event listener for input changes to filter and update URL | ||
// Debounce input to avoid frequent filtering | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you debounce the input? I don't think this is normally a problem? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. he was trying to make cypress work haha There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was trying to optimize some code because I thought the tests were failing because of me. I will remove the logic ASAP |
||
textBox.addEventListener("input", () => { | ||
const filterText = textBox.value; | ||
// Update the URL with the search parameter | ||
const newURL = new URL(window.location.href); | ||
newURL.searchParams.set("search", filterText); | ||
window.history.replaceState({}, "", newURL.toString()); | ||
|
||
// Filter the issues based on the input | ||
filterIssues(); | ||
if (this._filterTimeout) clearTimeout(this._filterTimeout); // Clear previous timeout | ||
this._filterTimeout = window.setTimeout(() => { | ||
const filterText = textBox.value; | ||
// Update the URL with the search parameter | ||
const newURL = new URL(window.location.href); | ||
newURL.searchParams.set("search", filterText); | ||
window.history.replaceState({}, "", newURL.toString()); | ||
filterIssues(); // Run the filter function | ||
}, 300); // 300ms debounce delay | ||
}); | ||
|
||
return textBox; | ||
|
@@ -131,31 +137,50 @@ export class SortingManager { | |
} | ||
|
||
private _handleSortingClick(input: HTMLInputElement, option: string) { | ||
const ordering = input === this._lastChecked ? "reverse" : "normal"; | ||
input.checked = input !== this._lastChecked; | ||
input.setAttribute("data-ordering", ordering); | ||
this._lastChecked = input.checked ? input : null; | ||
this._filterTextBox.value = ""; | ||
const currentOrdering = input.getAttribute("data-ordering"); | ||
let newOrdering: string; | ||
|
||
// Determine the new ordering based on the current state | ||
if (currentOrdering === "normal") { | ||
newOrdering = "reverse"; | ||
} else if (currentOrdering === "reverse") { | ||
newOrdering = "disabled"; | ||
} else { | ||
newOrdering = "normal"; | ||
} | ||
|
||
// Apply the new ordering state | ||
input.setAttribute("data-ordering", newOrdering); | ||
input.parentElement?.childNodes.forEach((node) => { | ||
if (node instanceof HTMLInputElement) { | ||
node.setAttribute("data-ordering", ""); | ||
} | ||
}); | ||
|
||
input.setAttribute("data-ordering", ordering); | ||
// instantly load from cache | ||
if (newOrdering === "disabled") { | ||
this._lastChecked = null; | ||
input.checked = false; | ||
this._clearSorting(); | ||
} else { | ||
input.checked = input !== this._lastChecked; | ||
this._lastChecked = input.checked ? input : null; | ||
input.setAttribute("data-ordering", newOrdering); | ||
|
||
// Apply the sorting based on the new state (normal or reverse) | ||
try { | ||
void displayGitHubIssues(option as Sorting, { ordering: newOrdering }); | ||
} catch (error) { | ||
renderErrorCatch(error as ErrorEvent); | ||
} | ||
} | ||
} | ||
|
||
private _clearSorting() { | ||
try { | ||
void displayGitHubIssues(option as Sorting, { ordering }); | ||
void displayGitHubIssues(); | ||
} catch (error) { | ||
renderErrorCatch(error as ErrorEvent); | ||
renderErrorInModal(error as Error); | ||
} | ||
// load from network in the background | ||
// const fetchedPreviews = await fetchIssuePreviews(); | ||
// const cachedTasks = taskManager.getTasks(); | ||
// const updatedCachedIssues = verifyGitHubIssueState(cachedTasks, fetchedPreviews); | ||
// displayGitHubIssues(sorting, options); | ||
// taskManager.syncTasks(updatedCachedIssues); | ||
// return fetchAvatars(); | ||
0x4007 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should remove the debounce logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, ill remove it