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

feat(i18n): add options field #478

Merged
merged 5 commits into from
May 11, 2020
Merged

feat(i18n): add options field #478

merged 5 commits into from
May 11, 2020

Conversation

mickr
Copy link
Contributor

@mickr mickr commented May 7, 2020

  • Add options field that can be passed into the instance. These options then will be passed to the annotator in Preview SDK. In this case, we will want to pass intl to the instance and then that should get passed to content preview, or which ever consumer calls determineAnnotator. These options will then be passed to the annotator configs in Preview SDK and use the current intl framework
  • Unit tests

@mickr mickr requested a review from a team as a code owner May 7, 2020 01:25
src/BoxAnnotations.ts Outdated Show resolved Hide resolved
@mickr mickr marked this pull request as draft May 7, 2020 18:21
@mickr mickr force-pushed the fix-intl-from-eua branch from e124c21 to 117fbf2 Compare May 8, 2020 16:52
Copy link
Collaborator

@jstoffan jstoffan left a comment

Choose a reason for hiding this comment

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

Approach seems good, given the limitations we're dealing with.

src/BoxAnnotations.ts Outdated Show resolved Hide resolved
@mickr mickr force-pushed the fix-intl-from-eua branch from 117fbf2 to b45425b Compare May 8, 2020 21:29
src/BoxAnnotations.ts Outdated Show resolved Hide resolved
@mickr mickr marked this pull request as ready for review May 9, 2020 00:15
mickr added 4 commits May 11, 2020 09:20
* Add options field that can be passed into the instance. These options then will be passed to the annotator in Preview SDK.
* Add language field in type
* Add intl wrapper object for options
@mickr mickr force-pushed the fix-intl-from-eua branch from 13d2f65 to 90ce21a Compare May 11, 2020 21:02
viewerOptions?: ViewerOptions | null;

/**
* [constructor]
*
* @param {Object} viewerOptions - Viewer-specific annotator options
* @param {AnnotationsOptions } options - options passed to the annotations instance
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: space between { and AnnotationsOptions

@@ -157,4 +157,17 @@ describe('BoxAnnotations', () => {
expect(loader.determineAnnotator(options, config)).toBeNull();
});
});

describe('getOptions', () => {
it.each([undefined, { intl: { messages: { test: 'Hello' } } }])(
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be test rather than it?

@mergify mergify bot merged commit 5e0bce0 into box:master May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants