Skip to content

Conversation

@cacheung
Copy link
Contributor

@cacheung cacheung commented Feb 18, 2022

Add Arraylist support in WriteableArray for Android.
If response payload has arraylist in it, it can be extracted now.

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Add Arraylist support in WriteableArray
} else if (value instanceof String) {
writableArr.pushString((String) value);
} else if (value instanceof Map) {
writableArr.pushMap(RCTAEPEdgeMapUtil.toWritableMap((Map<String, Object>) value));
Copy link
Contributor

Choose a reason for hiding this comment

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

we should add the check for instanceof ArrayList to L105 and L78 too

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we use Interface instead of concrete class here? (change it to value instanceof List ?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested with List and it also works. Seems like Changing to List is better?

Add ArrayList check for more locations.
@cacheung
Copy link
Contributor Author

@yangyansong-adbe @emdobrin Do we need to add ArrayList check for other extensions if they are using ArrayUtil?

@yangyansong-adbe
Copy link
Contributor

@cacheung can you check why CI is not triggered for this PR?

@yangyansong-adbe
Copy link
Contributor

@yangyansong-adbe @emdobrin Do we need to add ArrayList check for other extensions if they are using ArrayUtil?

@cacheung Yes, I think we need to update this utility method in other extensions.

Change checking the instanceof List instead of the ArrayList
@cacheung
Copy link
Contributor Author

@cacheung can you check why CI is not triggered for this PR?

CI is working now after @yangyansong-adbe enabled the setting for running CI for each PR.

@cacheung cacheung merged commit f7d0b58 into adobe:edge Feb 28, 2022
@cacheung cacheung deleted the EdgeResponse branch February 28, 2022 23:15
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