Update copy schedules #182
Conversation
| @@ -71,6 +71,12 @@ function App() { | |||
| //Don't set the term to WF, just keep the term to what is selected | |||
| setCurrentTerm(newData.terms.values().next().value) | |||
| } | |||
| } else if (changes.sections) { | |||
There was a problem hiding this comment.
listening to the storage change event here creates extra re-renders — like if we add a section to state + write it to storage, this will read it from storage + set state again, which is an unnecessary read+render cycle.
not a huge concern, but if you want to avoid it, probably a good idea to use a CustomEvent to tell the extension to refresh state instead.
There was a problem hiding this comment.
Makes sense, I am going to leave it out of this PR for the moment since it may affect potential state interactions. I will look into it further once this PR merges
src/content/utils.ts
Outdated
| @@ -48,7 +48,7 @@ async function extractSection(element: Element, bypassDetailsCheck?: boolean) { | |||
| } | |||
| } | |||
|
|
|||
| const extractIdFromDOM = (element: Element) => { | |||
| export const extractIdFromDOM = (element: Element) => { | |||
There was a problem hiding this comment.
update the export at the bottom of the file instead
| selectedSection = await findCourseInfo(code) | ||
| } | ||
| } catch { | ||
| continue |
There was a problem hiding this comment.
it would be good to store which sections couldn't be added, and alert the user at the end so they're aware of it
src/domManipulators/copySchedules.ts
Outdated
| // eslint-disable-next-line no-await-in-loop | ||
| const selectedSection = await findCourseInfo(code) | ||
| let selectedSection = null | ||
| console.log(tableData[i][tableData[i].length - 1]) |
There was a problem hiding this comment.
remove log? and the commented-out Browser imports in various places
What Issue does this PR resolve? (Link to GitHub Issue, approved features and bugs will be given priority)
Please provide a video demo below, or a screenshot and description of the change.
Tag reviewers for the PR below.