Implement k responsive window composable #377

akolson · 2022-10-06T15:05:51Z

Description

This pr ports the KResponsiveWindowMixin to useKResponsiveWindow composable. Logic has also been refactored to use the Window.matchMedia interface

Issue addressed

Closes #288

Before/after screenshots

No visual changes in products
KResponsiveWindow documentation
- before: https://design-system.learningequality.org/kresponsivewindow/
- after: https://deploy-preview-377--kolibri-design-system.netlify.app/kresponsivewindow/

Steps to test

Please follow the steps to test in Kolibri's corresponding PR Tests uKResponsiveWindow composable and Updates KDS Package to latest develop kolibri#9763
Read the updated documentation page and preview KResponsiveWindow for usage and examples

(optional) Implementation notes

At a high level, how did you implement this?

useKResponsiveWindow uses Window.matchMedia() to create reactive window properties including width, height, orientation etc

Does this introduce any tech-debt items?

Testing checklist

Contributor has fully tested the PR manually
If there are any front-end changes, before/after screenshots are included
Critical and brittle code paths are covered by unit tests
The change has been added to the changelog

Reviewer guidance

Is the code clean and well-commented?
Are there tests for this change?
Are all UI components LTR and RTL compliant (if applicable)?
Add other things to check for here

Post-merge updates and tracking

Learning Platform update PR is submitted
Learning Platform update PR is merged
Studio update PR is submitted
Studio update PR is merged
Data Portal update PR is submitted
Data Portal update PR is merged

Comments

…ames

… escaping

akolson · 2022-10-06T17:26:03Z

Hi @jtamiace, I am tagging you here so you could have a look at the documentation. Would appreciate your feedback on this. Thanks

bjester

Left some comments, but this looks awesome!

lib/useKResponsiveWindow.js

bjester · 2022-10-07T16:02:50Z

lib/useKResponsiveWindow.js

+      `(max-width: ${1440 - SCROLL_BAR}px)`,
+      `(max-width: ${1600 - SCROLL_BAR}px)`,
+      `(min-width: 1601px)`,
+    ];


The above 2 variables don't seem to change during calls to useKResponsiveWindow or widthQueries, so they could be moved further up in scope. Although I don't think it's a big deal.

lib/useKResponsiveWindow.js

docs/pages/kresponsivewindow.vue

lib/useKResponsiveWindow.js

jtamiace · 2022-10-07T17:50:36Z

@akolson the layout and format of the documentation additions look good to me! Would there be any value to engineers in making a short statement about which scenarios composable is preferable over mixin and vice versa? (I'm not an engineer, so if this is N/A or not helpful, please disregard)

akolson · 2022-10-07T17:58:56Z

@jtamiace you raise a valid question. More generally, composables are recommended. I will add a section to capture these thoughts. Thanks

…ortcomings in vue2

akolson · 2022-10-11T16:08:56Z

@bjester @rtibbles & @nucleogenesis, based on Friday feedback, some files were moved around, so incase you need to follow along based on old review, this link captures the previous code including your comments.

nucleogenesis

I gave the code a look and this looks great. I haven't tested it in Kolibri yet but will do so tomorrow and will give the docs a closer look

lib/useKWindowDimensions.js

bjester

This looks really great! The only blocker, and requested change, is that I think there's likely an issue with useKWindowDimensions-- please see my comment on that

lib/useKWindowDimensions.js

lib/useKResponsiveWindow/MediaQuery.js

nucleogenesis

With QA before me, Blaine & Richard's thoughts in mind; I've looked through the latest additions in the test suits and they're excellent - this should be good to merge!

commit f85ddfb Merge: 29122c4 9d29fe1 Author: akolson <[email protected]> Date: Thu Nov 3 22:42:28 2022 +0300 Merge pull request learningequality#380 from akolson/standardize-kradiobutton-labels Wraps KRadioButton label text commit 9d29fe1 Author: Samson Akol <[email protected]> Date: Wed Oct 26 23:34:47 2022 +0300 updates changelog commit ba2006c Author: Samson Akol <[email protected]> Date: Wed Oct 26 22:23:03 2022 +0300 updates documentation commit f083b25 Author: Samson Akol <[email protected]> Date: Wed Oct 26 22:04:02 2022 +0300 aligns kradiobutton api with kcheckbox's. Changes label from truncate to wrap. Adds truncateLabel prop to truncate text if necessary. updates documenation to reflect changes commit 29122c4 Merge: 115a19d 84ef450 Author: akolson <[email protected]> Date: Tue Oct 18 20:48:16 2022 +0300 Merge pull request learningequality#377 from akolson/implement-kResponsiveWindow-composable Implement k responsive window composable commit 84ef450 Author: Samson Akol <[email protected]> Date: Mon Oct 17 10:51:27 2022 +0300 Adds more review feedback commit 94abe9e Author: Samson Akol <[email protected]> Date: Fri Oct 14 19:16:35 2022 +0300 Improves test descriptions commit 6dc1d50 Author: Samson Akol <[email protected]> Date: Fri Oct 14 18:18:08 2022 +0300 Adds tests to useKWindowDimensions and useKResponsiveWindow commit 51d80a3 Author: Samson Akol <[email protected]> Date: Thu Oct 13 13:25:31 2022 +0300 minor string refactors commit 794ac2a Author: Samson Akol <[email protected]> Date: Thu Oct 13 12:55:00 2022 +0300 Fixes responsiveness bug on IE commit 445c740 Author: Samson Akol <[email protected]> Date: Wed Oct 12 23:01:34 2022 +0300 Adds feedback from review commit 68266f0 Author: Samson Akol <[email protected]> Date: Wed Oct 12 18:53:17 2022 +0300 Improves jsdoc, fixes IE issue commit b16c125 Author: Samson Akol <[email protected]> Date: Mon Oct 10 17:34:57 2022 +0300 Adds use composition-api to composable due to @vue/composition-api shortcomings in vue2 commit 4075c57 Author: Samson Akol <[email protected]> Date: Mon Oct 10 13:30:48 2022 +0300 Updates kresponsivewindow documentation commit 5940c76 Author: Samson Akol <[email protected]> Date: Mon Oct 10 12:38:45 2022 +0300 Adds feedback from code review commit b0fc205 Author: Samson Akol <[email protected]> Date: Thu Oct 6 18:27:24 2022 +0300 Updates changelog commit e504a70 Author: Samson Akol <[email protected]> Date: Thu Oct 6 15:52:29 2022 +0300 Code refactor commit 76c13a8 Author: Samson Akol <[email protected]> Date: Thu Oct 6 15:12:43 2022 +0300 Moves composition api initialization to useKResponsiveWindow commit 9dd5975 Author: Samson Akol <[email protected]> Date: Wed Oct 5 17:52:57 2022 +0300 Updates indentation commit 078c000 Author: Samson Akol <[email protected]> Date: Wed Oct 5 16:34:14 2022 +0300 uses anonymous function in documentation due to failures in character escaping commit ad681dd Author: Samson Akol <[email protected]> Date: Wed Oct 5 15:41:46 2022 +0300 Configures composition api, and documents the useKResponsiveWindow commit 2d1351e Author: Samson Akol <[email protected]> Date: Tue Oct 4 17:45:50 2022 +0300 Refactors code, adds documentation commit 8fe1519 Author: Samson Akol <[email protected]> Date: Mon Oct 3 18:59:22 2022 +0300 fixes bug in breakpoint media query listener commit f6afdf6 Author: Samson Akol <[email protected]> Date: Mon Oct 3 18:56:47 2022 +0300 Renames responsive window compodable to recommended composable file names commit c4278c2 Author: Samson Akol <[email protected]> Date: Fri Sep 30 23:15:15 2022 +0300 updates composable name commit fca8437 Author: Samson Akol <[email protected]> Date: Fri Sep 30 15:57:07 2022 +0300 Fixes bugs in composable commit e43b5a0 Author: Samson Akol <[email protected]> Date: Thu Sep 29 19:45:59 2022 +0300 Port of KResponsiveWindowMixin to KResponsiveWindow composable

akolson added 11 commits September 29, 2022 19:45

Port of KResponsiveWindowMixin to KResponsiveWindow composable

e43b5a0

Fixes bugs in composable

fca8437

updates composable name

c4278c2

Renames responsive window compodable to recommended composable file n…

f6afdf6

…ames

fixes bug in breakpoint media query listener

8fe1519

Refactors code, adds documentation

2d1351e

Configures composition api, and documents the useKResponsiveWindow

ad681dd

uses anonymous function in documentation due to failures in character…

078c000

… escaping

Updates indentation

9dd5975

Moves composition api initialization to useKResponsiveWindow

76c13a8

Code refactor

e504a70

akolson added the changelog label Oct 6, 2022

akolson mentioned this pull request Oct 6, 2022

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

Merged

9 tasks

Updates changelog

b0fc205

akolson requested review from bjester, jtamiace, nucleogenesis, radinamatic and rtibbles October 6, 2022 17:13

akolson added the TODO: needs review Waiting for review label Oct 6, 2022

akolson marked this pull request as ready for review October 6, 2022 17:24

bjester reviewed Oct 7, 2022

View reviewed changes

rtibbles reviewed Oct 7, 2022

View reviewed changes

lib/useKResponsiveWindow.js Outdated Show resolved Hide resolved

akolson added 2 commits October 10, 2022 12:38

Adds feedback from code review

5940c76

Updates kresponsivewindow documentation

4075c57

akolson requested review from bjester and rtibbles October 10, 2022 10:32

Adds use composition-api to composable due to @vue/composition-api sh…

b16c125

…ortcomings in vue2

rtibbles assigned nucleogenesis Oct 11, 2022

nucleogenesis reviewed Oct 11, 2022

View reviewed changes

lib/useKWindowDimensions.js Outdated Show resolved Hide resolved

akolson added 3 commits October 12, 2022 18:53

Improves jsdoc, fixes IE issue

68266f0

Adds feedback from review

445c740

Fixes responsiveness bug on IE

794ac2a

akolson requested a review from nucleogenesis October 13, 2022 10:18

akolson added 3 commits October 13, 2022 13:25

minor string refactors

51d80a3

Adds tests to useKWindowDimensions and useKResponsiveWindow

6dc1d50

Improves test descriptions

94abe9e

bjester requested changes Oct 14, 2022

View reviewed changes

lib/useKWindowDimensions.js Show resolved Hide resolved

lib/useKResponsiveWindow/MediaQuery.js Show resolved Hide resolved

lib/useKResponsiveWindow/MediaQuery.js Show resolved Hide resolved

rtibbles reviewed Oct 14, 2022

View reviewed changes

lib/useKResponsiveWindow/MediaQuery.js Outdated Show resolved Hide resolved

Adds more review feedback

84ef450

akolson requested review from bjester and rtibbles October 17, 2022 08:06

nucleogenesis approved these changes Oct 17, 2022

View reviewed changes

bjester approved these changes Oct 18, 2022

View reviewed changes

akolson merged commit 29122c4 into learningequality:develop Oct 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement k responsive window composable #377

Implement k responsive window composable #377

akolson commented Oct 6, 2022 •

edited

Loading

akolson commented Oct 6, 2022

bjester left a comment

bjester Oct 7, 2022

jtamiace commented Oct 7, 2022

akolson commented Oct 7, 2022

akolson commented Oct 11, 2022

nucleogenesis left a comment

bjester left a comment

nucleogenesis left a comment

Implement k responsive window composable #377

Implement k responsive window composable #377

Conversation

akolson commented Oct 6, 2022 • edited Loading

Description

Issue addressed

Before/after screenshots

Steps to test

(optional) Implementation notes

At a high level, how did you implement this?

Does this introduce any tech-debt items?

Testing checklist

Reviewer guidance

Post-merge updates and tracking

Comments

akolson commented Oct 6, 2022

bjester left a comment

Choose a reason for hiding this comment

bjester Oct 7, 2022

Choose a reason for hiding this comment

jtamiace commented Oct 7, 2022

akolson commented Oct 7, 2022

akolson commented Oct 11, 2022

nucleogenesis left a comment

Choose a reason for hiding this comment

bjester left a comment

Choose a reason for hiding this comment

nucleogenesis left a comment

Choose a reason for hiding this comment

akolson commented Oct 6, 2022 •

edited

Loading