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

pkp/pkp-lib#9950 Add laravel pagination support for useFetchPaginated #348

Merged
merged 1 commit into from
May 21, 2024

Conversation

jardakotesovec
Copy link
Collaborator

No description provided.

@@ -15,6 +15,7 @@ import {setActivePinia, createPinia} from 'pinia';

import {useFetch} from './useFetch';
import {useModalStore} from '@/stores/modalStore';
global.pkp = global.pkp || {};
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this not causing issues in previous versions of the test or is there something new with the new tests that necessitated this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ewhanson yeah, we have not had ui-library unit tests running on CI - so it regressed over time..

We are adding the it here pkp/pkp-lib#9951

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome, that's great!

@@ -16,7 +16,7 @@
export function t(key, params) {
if (
typeof pkp === 'undefined' ||
typeof pkp.localeKeys[key] === 'undefined'
typeof pkp?.localeKeys?.[key] === 'undefined'
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this and the other optional chaining additions causing issues previously, or was there something specific to these changes that drew your attention to this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was causing issues previously.. but noticed it while working on this, when I run tests after some time.

Copy link
Contributor

Choose a reason for hiding this comment

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

Figured it was something like this, but wanted to make sure I wasn't missing anything else. Thanks!

@jardakotesovec jardakotesovec merged commit 3da58dc into pkp:main May 21, 2024
4 checks passed
@jardakotesovec jardakotesovec deleted the i9950 branch May 21, 2024 07:46
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