Skip to content
New issue

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

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

Already on GitHub? # to your account

Delete coreAPI PageRoot component in favour of a default component in kolibri_app #11730

Merged

Conversation

rtibbles
Copy link
Member

@rtibbles rtibbles commented Jan 12, 2024

Summary

Instead of the core API exposing a very thin PageRoot component that is just a wrapper around router-view, instead make an even thinner component that is just used by default.

References

Fixes #11729

Reviewer guidance

All affected pages should still render identically.


Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

PR process

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@rtibbles rtibbles added the TODO: needs review Waiting for review label Jan 12, 2024
@rtibbles rtibbles requested a review from marcellamaki January 12, 2024 22:26
@github-actions github-actions bot added APP: Learn Re: Learn App (content, quizzes, lessons, etc.) APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) APP: User Re: User app (sign-in, sign-up, user profile, etc.) DEV: frontend SIZE: small labels Jan 12, 2024
Copy link

@cbum-dev cbum-dev left a comment

Choose a reason for hiding this comment

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

Reviewed two changes.

@marcellamaki marcellamaki self-assigned this Jan 16, 2024
Copy link
Member

@marcellamaki marcellamaki left a comment

Choose a reason for hiding this comment

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

lgtm - manual QA confirms affected pages still render as previously

@marcellamaki
Copy link
Member

Merging without Python unit tests on Mac OS (3.10), as this is a side effect of the recently updated check to run Python unit tests on Mac OS using 3.10, and the previously existing check (when this PR was opened, 3.6) passed.

@marcellamaki marcellamaki merged commit 51a7503 into learningequality:release-v0.16.x Feb 27, 2024
34 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) APP: Learn Re: Learn App (content, quizzes, lessons, etc.) APP: User Re: User app (sign-in, sign-up, user profile, etc.) DEV: frontend SIZE: small TODO: needs review Waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace instances of the PageRoot component with a default RootVue component
3 participants