Skip to content

Conversation

@lposen
Copy link
Contributor

@lposen lposen commented Oct 1, 2024

🔹 JIRA Ticket(s) if any

✏️ Description

  • Fixes lint issues and ts issues
  • Adds necessary libs to package.json
  • Ignores folders if we don't currently care to lint them as they will either be significantly changed or deleted. The ignores will be removed in later PRs.

This commit refactors the `IterableAction` and `IterableActionContext` classes. It moves them from the `src` folder to the `ts` folder. Additionally, it removes the unused `use strict` directive and renames the `index` file. The changes aim to improve the code organization and maintainability.
Refactor IterableInbox.tsx and remove unused imports

Add comment to IterableInboxMessageCell.tsx

Add comment to IterableInboxMessageDisplay.tsx

Add comments and descriptions to IterableLogger.ts

Add a description to IterableUtil.ts

Add comment to useAppStateListener.ts

Add comment to useDeviceOrientation.tsx
Refactor IterableInbox.tsx and improve code readability
}
)
useEffect(() => {
const listener = AppState.addEventListener(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a change needed due to the react native upgrade

@lposen lposen requested review from Ayyanchira and evantk91 October 31, 2024 03:51
Base automatically changed from 2.0.0-alpha/MOB-9678-fix-build to 2.0.0-alpha/master November 14, 2024 21:49
Copy link
Contributor

@evantk91 evantk91 left a comment

Choose a reason for hiding this comment

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

Some clarifications. Mostly pointed out that we have an export statement at the end of alot of files that seem redundant but this needs to be verified.

"react-native-builder-bob": "^0.30.2",
"react-native-safe-area-context": "^4.11.1",
"react-native-vector-icons": "^10.2.0",
"react-native-webview": "^13.12.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting so did the linting add these libraries?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The linting showed that they were unresolved, so I added them.

'use strict'
/**
* TODO: Split into seperate files
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea.

NativeModules,
NativeEventEmitter,
Linking,
Platform,
Copy link
Contributor

Choose a reason for hiding this comment

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

This ending comma seems weird. Is this the convention?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can specify it in the prettier, but most Frontend libs have the ending comma. The reason is that it makes sorting easier.

inApp = 2,
}

enum AuthResponseCallback {
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 exported?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also needs a description. Should add a to-do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. Will do!

class IterableActionContext {
action: IterableAction
source: IterableActionSource
export class IterableActionContext {
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like all of these should have descriptions. Can be a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah def! I have a ticket for it. Reorganizing things and removing dupes first.

}
}

export default IterableUtil;
Copy link
Contributor

Choose a reason for hiding this comment

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

Redundant.

@@ -1,10 +1,7 @@

enum IterablePushPlatform {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe have export here to align with other files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!

}

export { IterableLogger }
export default IterableLogger;
Copy link
Contributor

Choose a reason for hiding this comment

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

Redundant.

};

export default IterableInboxMessageList
export default IterableInboxMessageList;
Copy link
Contributor

Choose a reason for hiding this comment

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

Redundant.

);
};

export default IterableInboxMessageDisplay;
Copy link
Contributor

Choose a reason for hiding this comment

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

same.

@lposen lposen merged commit 291884c into 2.0.0-alpha/master Nov 15, 2024
1 of 3 checks passed
@lposen lposen deleted the 2.0.0-alpha/MOB-9671-fix-linting-issues branch November 15, 2024 02:29
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