-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/example refactor #2
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
Conversation
…onents - Simplified App component by extracting header, payment event, loading indicator, payment buttons, and transaction info into separate components. - Introduced a theme for consistent styling across the application. - Created a Button component for reusable button styles and states. - Added a Card component for consistent card styling. - Implemented a LoadingIndicator component for better loading state management. - Created PaymentButtons component to handle payment actions. - Added PaymentEvent component to display payment event messages. - Introduced TransactionInfo component to show details of the last transaction. - Refactored payment operations into a custom hook for better state management and separation of concerns. - Added utility functions for alert handling and async operations. - Defined constants for payment amounts and terminal activation code.
- Removed predefined themes from the library and transitioned to a fully customizable theme approach. - Updated README and STYLING_GUIDE to reflect new theme creation methods and examples. - Simplified theme application in example app, ensuring themes are applied on initialization. - Enhanced theme utility functions for better validation and merging of themes. - Cleaned up example components to focus on dynamic theme switching and customization. - Removed obsolete StyleExampleNew component to streamline codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces UI theming capabilities to the PagBank SDK integration and refactors the example app for better code organization. It adds a complete theme customization system that allows apps to match PagBank modal dialogs to their design system, while also restructuring the example code into reusable components and utilities.
- Adds UI theming API with
setStyleTheme
function and theme validation utilities - Refactors example app into modular components and custom hooks
- Updates native Android code with improved error handling and styling support
Reviewed Changes
Copilot reviewed 30 out of 31 changed files in this pull request and generated 7 comments.
Show a summary per file
File | Description |
---|---|
src/themes.ts | New theme utility class with validation and merging functions |
src/index.tsx | Exports new theming functionality and updates API signatures |
src/PlugpagNitro.nitro.ts | Adds PlugpagStyleData interface and theming method signatures |
package.json | Updates Jest and TypeScript dependencies |
example/src/utils/* | New utility modules for payment operations, alerts, and async handling |
example/src/hooks/* | Custom hook for payment operations state management |
example/src/constants/* | Theme constants and design system |
example/src/components/* | Modular UI components for the example app |
example/src/App.tsx | Simplified main component using new modular architecture |
android/.../PlugpagNitro.kt | Refactored with improved error handling and theming support |
android/.../*Manager.kt | New utility classes for styling, operations, and result building |
): Promise<ErrorCode> { | ||
return safeModuleCall('print', () => | ||
PlugpagNitroModule.print(filePath, textSize) | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return type change for the print function from Promise to Promise is a breaking change. This affects any existing code that relies on the print function not returning a value.
): Promise<ErrorCode> { | |
return safeModuleCall('print', () => | |
PlugpagNitroModule.print(filePath, textSize) | |
); | |
): Promise<void> { | |
const errorCode = await safeModuleCall('print', () => | |
PlugpagNitroModule.print(filePath, textSize) | |
); | |
if (errorCode !== ErrorCode.SUCCESS) { | |
console.error(`Print failed with error code: ${errorCode}`); | |
} |
Copilot uses AI. Check for mistakes.
return safeModuleCall('print', () => PlugpagNitroModule.print(filePath)); | ||
export async function print( | ||
filePath: string, | ||
textSize: number = 20 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The magic number '20' for the default text size should be defined as a named constant to improve maintainability and make it easier to change globally.
textSize: number = 20 | |
textSize: number = DEFAULT_TEXT_SIZE |
Copilot uses AI. Check for mistakes.
/** | ||
* Terminal activation code | ||
*/ | ||
export const TERMINAL_ACTIVATION_CODE = '403938'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The terminal activation code is hardcoded and exposed in the example. This could be a security concern if this code is used in production. Consider adding a comment that this is for demo purposes only.
Copilot uses AI. Check for mistakes.
fun createStyleDataFromNitro(nitroStyleData: com.margelo.nitro.plugpagnitro.PlugpagStyleData): PlugPagStyleData { | ||
return try { | ||
// Use reflection to call setStyleData method | ||
val plugPagClass = Class.forName("br.com.uol.pagseguro.plugpagservice.wrapper.PlugPag") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using hardcoded class name strings with reflection makes the code fragile to SDK changes. Consider using a more robust approach or at least extracting the class name to a constant.
val plugPagClass = Class.forName("br.com.uol.pagseguro.plugpagservice.wrapper.PlugPag") | |
val plugPagClass = Class.forName(PLUG_PAG_CLASS_NAME) |
Copilot uses AI. Check for mistakes.
|
||
@DoNotStrip | ||
class PlugpagNitro : HybridPlugpagNitroSpec() { | ||
|
||
private lateinit var plugPag: PlugPag | ||
private val eventContext = mutableMapOf<String, Any>() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mutableMapOf for eventContext could have thread safety issues if accessed concurrently during payment operations. Consider using a thread-safe collection or proper synchronization.
Copilot uses AI. Check for mistakes.
This pull request introduces a comprehensive UI customization and theming feature for the PagBank SDK, along with improvements to the Android codebase for better error handling, event management, and maintainability. The changes include a new theming API, updated documentation, and utility classes for consistent error and event handling.
UI Customization and Theming:
setStyleTheme
method and utilities for theme validation and merging. (README.md
,STYLING_GUIDE.md
) [1] [2]STYLING_GUIDE.md
) with examples for creating dark, light, and brand-based themes, along with best practices for accessibility and error handling.Android Codebase Enhancements:
ErrorCodeMapper
to map PlugPag SDK error codes to Nitro-specific error codes, reducing duplication. (ErrorCodeMapper.kt
)OperationHandler
for consistent error handling and context switching in asynchronous operations, eliminating repetitive try/catch blocks. (OperationHandler.kt
)PaymentEventHandler
for configuration-driven payment event processing, simplifying complex switch statements with pattern-based mappings. (PaymentEventHandler.kt
)PlugpagNitro.kt
)These changes improve the SDK's flexibility, maintainability, and developer experience, enabling seamless integration and customization for client applications.