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

Tests uKResponsiveWindow composable and Updates KDS Package to latest develop #9763

Merged

Conversation

akolson
Copy link
Member

@akolson akolson commented Oct 6, 2022

Summary

This pr implements the useKResponsiveWindow composable in areas where its mixin equivalent(KResponsiveWindowMixin) is used particularly in kolibri/plugins/learn/assets/src/views/LibraryPage/SidePanel.vue and kolibri/plugins/learn/assets/src/views/LibraryPage/index.vue as a test.

Please note that my KDS fork has been referenced for testing purposes. This will be reverted once KDS PR learningequality/kolibri-design-system#377 gets merged

References

learningequality/kolibri-design-system#377

Reviewer guidance

  1. Navigate to http://localhost:8000/en/learn/#/library
  2. Test the window for responsiveness. It should behave as before when using the mixin

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

@akolson akolson added the changelog Important user-facing changes label Oct 6, 2022
@akolson akolson added this to the Planned Patch 6 milestone Oct 6, 2022
@akolson akolson changed the title Tests uKResponsiveWindow to implement responsive, replacing the mixin… Tests uKResponsiveWindow composable Oct 6, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2022

@akolson akolson changed the title Tests uKResponsiveWindow composable Tests uKResponsiveWindow composable and Updates KDS Package to latest develop Oct 6, 2022
@akolson akolson added the TODO: needs review Waiting for review label Oct 6, 2022
@akolson akolson marked this pull request as ready for review October 6, 2022 17:26
@radinamatic
Copy link
Member

radinamatic commented Oct 11, 2022

Tested by running from the source with Firefox on Ubuntu, no regressions on the Learn/Library page when resizing the window, neither manually nor with the Responsive Design Mode.

@pcenov Could you grab the Windows asset and test with Chrome? Thank you!

@rtibbles rtibbles modified the milestones: Planned Patch 6, 0.16.0 Oct 11, 2022
@pcenov
Copy link
Member

pcenov commented Oct 12, 2022

Hi @akolson and @radinamatic the main regression I see is that the Library page cannot be opened any more in Internet Explorer:

2022-10-12_14-46-06

I couldn't find any other issues in the rest of the supported browsers, but I do have plenty reported here: #9726

@radinamatic
Copy link
Member

Library page cannot be opened any more in Internet Explorer

Oouch, thank you for fishing that out, I tend to forget IE11 (wishful thinking, I know 😒)

@akolson
Copy link
Member Author

akolson commented Oct 12, 2022

Great catch @pcenov. Fixing this. Thanks

@akolson
Copy link
Member Author

akolson commented Oct 12, 2022

@radinamatic @pcenov. The issue with IE should be resolved now!

@pcenov
Copy link
Member

pcenov commented Oct 13, 2022

@akolson I confirm that now the Library page can be opened in IE however the responsiveness is not functioning correctly. For example if I shrink the window while at the Library page the filters section never goes in mobile view, while if I go in mobile view and then expand the window the filters section doesn't go back to desktop mode:

2022-10-13_11-03-12.mp4

@akolson
Copy link
Member Author

akolson commented Oct 13, 2022

Hi @pcenov, Thanks for the feedback. I have fixed the issues with responsiveness on IE so we should be good now!

@pcenov
Copy link
Member

pcenov commented Oct 13, 2022

Awesome @akolson - everything looks fine now in all supported browsers! :)

Copy link
Member

@radinamatic radinamatic left a comment

Choose a reason for hiding this comment

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

Manual QA passes, good to go! 💯 :shipit:

@akolson
Copy link
Member Author

akolson commented Oct 18, 2022

PR has been updated to the latest KDS commit

@akolson akolson merged commit 0958053 into learningequality:develop Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog Important user-facing changes TODO: needs review Waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants