Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Add logic to switch between sections in navigation bar - #60537

#60537
Merged
lfryemason merged 8 commits into
stagingcode-dot-org/code-dot-org:stagingfrom
lfm/n-loadercode-dot-org/code-dot-org:lfm/n-loaderCopy head branch name to clipboard
Aug 22, 2024
Merged

Add logic to switch between sections in navigation bar#60537
lfryemason merged 8 commits into
stagingcode-dot-org/code-dot-org:stagingfrom
lfm/n-loadercode-dot-org/code-dot-org:lfm/n-loaderCopy head branch name to clipboard

Conversation

@lfryemason

@lfryemason lfryemason commented Aug 20, 2024

Copy link
Copy Markdown
Contributor

Adds logic to switch sections using react-router. This is done by using a loader that will fetch data from a new endpoint /dashboardapi/section/:section_id about the selected section. A loader is a <Route> prop function that gets called whenever the url changes to that <Route>. This is also called on initial load, so we don't need to set anything that will be set by this function.

Additions:

  • Add a new endpoint to load selected section data
  • Add a react-router loader to the section path that calls the new endpoint and updates the redux store
    • Redux updates are either in the loader or for the v1
  • Use useNavigate and useLocation to switch paths for sections and pages from the sidebar.
  • Add tests for switching pages in the sidebar

Follow ups needed:

  • Move anyStudentHasProgress to the selected section data and into redux - this is necessary so that switching to a different section will not show the same error if the first section does not have students

Links

TEACH-1236

@lfryemason
lfryemason marked this pull request as ready for review August 21, 2024 20:40
@lfryemason
lfryemason requested a review from kobryan0619 August 21, 2024 20:40
sectionProviderName,
setSections,
setShowLockSectionField,
setStudentsForCurrentSection, // DCDO Flag - show/hide Lock Section field

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this comment got moved to the wrong line?

await screen.findByText(i18n.classSections());
screen.getByRole('combobox');
screen.getByText('Period 1');
expect(screen.getByText('Period 1')).toBeVisible();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I ask why you added the .toBeVisible line here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very slight improvement, this tests that the period 1 is visible and selected in the dropdown. The other options should be invisible because the dropdown is not open.

@lfryemason
lfryemason merged commit 3a725e4 into staging Aug 22, 2024
@lfryemason
lfryemason deleted the lfm/n-loader branch August 22, 2024 13:20
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.