Skip to content

(DOCSP-26984): @realm/reactify: Mixed - React Native SDK #2411

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

Merged

Conversation

mohammadhunan-dev
Copy link
Contributor

@mohammadhunan-dev mohammadhunan-dev commented Dec 15, 2022

Pull Request Info

Jira

Staged Changes

Reminder Checklist

If your PR modifies the docs, you might need to also update some corresponding
pages. Check if completed or N/A.

  • [N/A] Create Jira ticket for corresponding docs-app-services update(s), if any
  • [N/A] Checked/updated Admin API
  • [N/A] Checked/updated CLI reference

Review Guidelines

REVIEWING.md

@github-actions
Copy link

github-actions bot commented Dec 15, 2022

Readability for Commit Hash: 3c5f807

You can see any previous Readability scores (if they exist) by looking
at the comment's history.

Flesch Reading Ease scores for changed documents:

  • source/sdk/react-native/realm-database/schemas/mixed: 65.01

The following table can be helpful in assessing the readability score of a document.

Score Difficulty
90-100 Very Easy
80-89 Easy
70-79 Fairly Easy
60-69 Medium
50-59 Fairly Hard
30-49 Hard
0-29 Very Hard

@mohammadhunan-dev mohammadhunan-dev changed the title wip - mixed tests wip - (DOCSP-26984): @realm/reactify: Mixed - React Native SDK Dec 15, 2022
@mohammadhunan-dev mohammadhunan-dev changed the title wip - (DOCSP-26984): @realm/reactify: Mixed - React Native SDK (DOCSP-26984): @realm/reactify: Mixed - React Native SDK Dec 16, 2022
@@ -0,0 +1,145 @@
import React, {useEffect} from 'react';
Copy link
Collaborator

Choose a reason for hiding this comment

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

all comments/suggestions on examples/react-native/__tests__/js/realm-database/schemas/mixed-tests.jsx also apply to this file

Copy link
Collaborator

Choose a reason for hiding this comment

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

Comment on lines 17 to 19
type except a collection. You can create collections (lists, sets, and
dictionaries) of type ``mixed``, but a ``mixed`` type itself cannot be a
collection. Properties using the mixed data type can also hold null values.
Copy link
Collaborator

@mongodben mongodben Dec 16, 2022

Choose a reason for hiding this comment

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

outside of scope of this PR, but would be good to have example of using mixed type with a collection.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting idea, I'm not opposed to it, but don't necessarily want to bulk up this page. Maybe a note saying for additional examples, see ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

what would that note point to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possibly an "Additional Examples" page like we used to have for C#?

Copy link
Collaborator

@mongodben mongodben left a comment

Choose a reason for hiding this comment

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

nice work! i like the way you handled the @realm/react-ification of the examples.

couple small suggestions/nits throughout.

and then some larger comments about how the data type is described that fall outside of the scope of this PR, but should probably be addressed. could be good work for @krollins-mdb to learn more about the SDK.

@mohammadhunan-dev mohammadhunan-dev changed the base branch from master to realm-react-guidance December 27, 2022 23:07
Comment on lines +48 to +50

useEffect(() => {
// Add data to the Realm when the component mounts
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
useEffect(() => {
// Add data to the Realm when the component mounts
// Add data to the Realm when the component mounts
useEffect(() => {

nit: think it'd be clearer if this comment before the hook.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like it where it is because the comment is directly above the write transaction where you add the data

Copy link
Collaborator

@mongodben mongodben left a comment

Choose a reason for hiding this comment

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

lgtm.

couple little things to fix and 1 open question, but nothing blocking.

@mohammadhunan-dev mohammadhunan-dev merged commit 6e17388 into mongodb:realm-react-guidance Jan 4, 2023
mongodben added a commit that referenced this pull request Feb 10, 2023
## Pull Request Info

### Jira

- https://jira.mongodb.org/browse/DOCSP-26984

### Staged Changes

- [Mixed - React Native
SDK](https://docs-mongodbcom-staging.corp.mongodb.com/realm/docsworker-xlarge/mixed-object/sdk/react-native/realm-database/schemas/mixed/)

### Reminder Checklist

If your PR modifies the docs, you might need to also update some
corresponding
pages. Check if completed or N/A.

- [N/A] Create Jira ticket for corresponding docs-app-services
update(s), if any
- [N/A] Checked/updated Admin API
- [N/A] Checked/updated CLI reference

### Review Guidelines


[REVIEWING.md](https://github.com/mongodb/docs-realm/blob/master/REVIEWING.md)

Co-authored-by: Mohammad Hunan Chughtai <[email protected]>
Co-authored-by: Ben Perlmutter <[email protected]>
Copy link
Collaborator

@takameyer takameyer left a comment

Choose a reason for hiding this comment

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

What we are missing here is a nice method called Realm.Object.getPropertyType(propertyName:string). This will return the underlying property type as a string. This is useful when trying to do some type checking on a mixed property.
The cat example could use this to logically depict how it should interpret the given birthday type.

mongodben added a commit that referenced this pull request Mar 21, 2023
## Pull Request Info

### Jira

- https://jira.mongodb.org/browse/DOCSP-26984

### Staged Changes

- [Mixed - React Native
SDK](https://docs-mongodbcom-staging.corp.mongodb.com/realm/docsworker-xlarge/mixed-object/sdk/react-native/realm-database/schemas/mixed/)

### Reminder Checklist

If your PR modifies the docs, you might need to also update some
corresponding
pages. Check if completed or N/A.

- [N/A] Create Jira ticket for corresponding docs-app-services
update(s), if any
- [N/A] Checked/updated Admin API
- [N/A] Checked/updated CLI reference

### Review Guidelines


[REVIEWING.md](https://github.com/mongodb/docs-realm/blob/master/REVIEWING.md)

Co-authored-by: Mohammad Hunan Chughtai <[email protected]>
Co-authored-by: Ben Perlmutter <[email protected]>
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.

3 participants