Payment QOL #273
Payment QOL #273
Conversation
- update golden token collection link - add new sound tracks for spawning adventurer - add dynamic volumes for sounds and music - add new payment page
- add larger shadow
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes focus on the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Outside diff range and nitpick comments (18)
ui/src/app/hooks/useUiSound.tsx (2)
16-25: Volume map provides fine-grained control. Consider using constants for common values.The introduction of the
volumeMapobject is a good addition for controlling individual sound effect volumes. For improved maintainability, consider using constants for common volume levels:const STANDARD_VOLUME = 0.2; const LOUD_VOLUME = 0.4; const QUIET_VOLUME = 0.1; const volumeMap = { click: LOUD_VOLUME, discoverItem: STANDARD_VOLUME, flee: STANDARD_VOLUME, jump: STANDARD_VOLUME, slay: STANDARD_VOLUME, hit: STANDARD_VOLUME, coin: STANDARD_VOLUME, spawn: QUIET_VOLUME, };This approach makes it easier to adjust volumes consistently across multiple sound effects.
31-31: Volume setting updated to use volumeMap. Consider using type assertion for better type safety.The update to use
volumeMapwith a fallback todefaultVolumeis well implemented. For improved type safety, consider using a type assertion instead of a type cast:volume: volumeMap[selector as keyof typeof volumeMap] ?? defaultVolume,This change ensures that
selectoris a valid key ofvolumeMapat compile-time, reducing the chance of runtime errors.ui/src/app/hooks/useMusic.tsx (3)
7-10: LGTM! Consider using constants for volume values.The updated
musicSelectorobject now includes both track and volume information for each music type, which is a good improvement for granular audio control.Consider defining constants for the volume values (e.g.,
BACKGROUND_VOLUME,BATTLE_VOLUME,DEATH_VOLUME) at the top of the file. This would make it easier to adjust and maintain volume levels across different tracks.Example:
const BACKGROUND_VOLUME = 0.5; const BATTLE_VOLUME = 0.5; const DEATH_VOLUME = 0.75; export const musicSelector = { backgroundMusic: { track: "intro.mp3", volume: BACKGROUND_VOLUME }, battle: { track: "fight4.mp3", volume: BATTLE_VOLUME }, battle2: { track: "fight5.mp3", volume: BATTLE_VOLUME }, death: { track: "game_over.mp3", volume: DEATH_VOLUME }, };
22-24: LGTM! Consider destructuring for improved readability.The
useSoundhook usage has been correctly updated to use the newmusicstate structure, properly applying both the track and volume.For improved readability, consider destructuring the
musicobject:const [play, { stop }] = useSound(dir + music.track, { volume: music.volume, loop: options?.loop ?? false, });This change makes the code slightly more concise and easier to read. Additionally, using the nullish coalescing operator (
??) instead of logical OR (||) for theloopoption would be more precise, as it only falls back tofalseifoptions?.loopisnullorundefined, not for all falsy values.
Line range hint
1-67: Overall, great improvements to the music system!The changes in this file enhance the music system by allowing more granular control over individual track volumes. The updates to the
musicSelectorobject, state initialization, anduseSoundhook usage are consistent and maintain type safety. These improvements will likely lead to a better audio experience for users, aligning well with the PR's objective of enhancing the overall user experience.Consider adding a configuration file for music tracks and their default volumes. This would make it easier to manage and update the audio settings without modifying the core logic.
ui/tailwind.config.js (1)
69-69: Approved: New box shadow configuration enhances styling options.The addition of the
xlbox shadow configuration is a good enhancement to the existing styling options. It provides a larger, more pronounced shadow effect that can be useful for highlighting important elements, potentially improving the visual hierarchy on the payment page.The use of the semi-transparent green color (rgb(51, 255, 51) with 50% opacity) is consistent with the existing color scheme, which is good for maintaining design coherence.
For consistency with other color definitions in this file, consider defining this color as a named color in the
colorssection. For example:colors: { // ... existing colors ... "terminal-green-50-bright": "rgb(51 255 51 / 0.5)", },Then you could use it in the box shadow definition:
xl: "0px 10px 40px 10px var(--terminal-green-50-bright)",This approach would make it easier to maintain consistent colors across the application and improve readability.
ui/src/app/lib/networkConfig.ts (2)
60-63: LGTM: Updates to goldenTokenMintUrl and addition of adventurerViewer for mainnetThe changes to the mainnet configuration are appropriate:
- The
goldenTokenMintUrlnow points to a specific collection, which should provide more precise information to users.- The addition of the
adventurerViewerproperty is consistent with other environments and enhances the user experience.These updates align well with the PR objectives of improving payment information clarity and enhancing the Adventurer NFT minting process.
Consider adding a comment explaining the significance of the collection IDs used in these URLs, as it might be helpful for future maintenance.
114-115: LGTM with suggestion: Addition of adventurerViewer for localKatana environmentThe addition of the
adventurerViewerproperty in the localKatana configuration maintains consistency with other environments, which is good for code uniformity.However, given that localKatana appears to be a local development environment (based on the use of localhost URLs for other properties), it might be more appropriate to use a local or mock URL for the
adventurerVieweras well. This would allow for easier testing and development without relying on external services.Consider updating the
adventurerViewerURL to a local address, for example:- adventurerViewer: - "https://market.realms.world/collection/0x018108b32cea514a78ef1b0e4a0753e855cdf620bc0565202c02456f618c4dc4", + adventurerViewer: "http://localhost:3000/mock-adventurer-viewer",This change would make the localKatana environment more self-contained and easier to work with during local development.
ui/src/app/globals.css (1)
16-18: LGTM! Consider adding a comment for clarity.The new
.no-text-shadowclass is a simple and effective way to remove text shadows from specific elements. This can improve readability where needed, which aligns with the PR's goal of enhancing the payment page.Consider adding a brief comment above the class to explain its purpose:
+/* Utility class to remove text shadow */ .no-text-shadow { text-shadow: none; }This will help other developers understand the intention behind this class quickly.
ui/src/app/components/start/PaymentDetails.tsx (2)
11-137: LGTM: Well-structured component with consistent styling.The
PaymentTablecomponent is well-organized, uses Tailwind CSS effectively, and handles missing data gracefully. Good job on the overall implementation.Consider extracting hardcoded values.
The component contains several hardcoded values for prices and payouts. Consider extracting these into constants or a configuration object for better maintainability and easier updates.
Here's an example of how you could refactor this:
const GAME_PRICES = { baseFee: 3.00, randomness: 0.50, }; const PAYOUTS = { first: 0.81, second: 0.48, third: 0.30, clientProvider: 0.81, creator: 0.60, }; // Then use these in your component, e.g.: <p className="text-lg">${GAME_PRICES.baseFee.toFixed(2)}</p>Enhance accessibility with ARIA attributes.
To improve the component's accessibility, consider adding appropriate ARIA attributes. For example:
- <div className="flex flex-col border border-terminal-green b-5 bg-terminal-black uppercase w-full"> + <div className="flex flex-col border border-terminal-green b-5 bg-terminal-black uppercase w-full" role="table" aria-label="Game Prices and Payouts"> - <h1 className="m-0 p-2 text-lg sm:text-2xl">Game Prices</h1> + <h1 className="m-0 p-2 text-lg sm:text-2xl" role="rowgroup" aria-label="Game Prices Section">Game Prices</h1> - <div className="flex flex-row w-full justify-between p-2 border-t border-terminal-green"> + <div className="flex flex-row w-full justify-between p-2 border-t border-terminal-green" role="row"> - <div className="flex flex-row gap-2 items-center"> + <div className="flex flex-row gap-2 items-center" role="cell"> <p className="sm:text-2xl">Base Fee</p> </div> - <div className="flex flex-row gap-2 items-center"> + <div className="flex flex-row gap-2 items-center" role="cell"> ... </div> </div> </div>
139-161: LGTM: Well-structured main component with responsive design.The
PaymentDetailscomponent is well-implemented, using the defined props interface and providing responsive rendering for different screen sizes.Consider refactoring to reduce duplication.
The component renders
PaymentTabletwice with similar wrapping divs. This could be refactored to reduce code duplication.Here's a suggested refactor:
const PaymentDetails = ({ adventurers, showPaymentDetails, }: PaymentDetailsProps) => { const paymentTable = <PaymentTable adventurers={adventurers} />; return ( <> <div className="hidden sm:flex flex-col items-center justify-center sm:gap-10 gap-5 w-full no-text-shadow"> {paymentTable} </div> {showPaymentDetails && ( <div className="flex flex-col items-center justify-center sm:gap-10 gap-5 w-full no-text-shadow"> {paymentTable} </div> )} </> ); };This approach creates the
PaymentTableonce and reuses it, reducing duplication and potentially improving performance.ui/src/app/components/adventurer/NftCard.tsx (1)
1-392: Overall assessment: Good foundation, needs refinement for production useThe
NftCardcomponent provides a solid foundation for rendering NFT details in an SVG format. However, to make it production-ready and more maintainable, consider the following key improvements:
- Implement dynamic data rendering throughout the component.
- Extract repeated patterns into reusable sub-components.
- Use props for all variable data instead of hardcoding values.
- Optimize the layout for better scalability and responsiveness.
- Consider extracting SVG paths and styles into separate files for better organization.
These changes will significantly enhance the component's flexibility, maintainability, and reusability.
ui/src/app/lib/utils/syscalls.ts (5)
Line range hint
433-1235: Consider refactoring thecreateSyscallsfunction for improved maintainabilityThe
createSyscallsfunction is quite large and complex, handling various game actions such as spawning, exploring, attacking, fleeing, upgrading, and more. While it's functional, its size and complexity could make it difficult to maintain and test in the long run.Consider the following refactoring suggestions:
Split the function into smaller, more focused functions. Each game action (spawn, explore, attack, etc.) could be its own function.
Use a command pattern or strategy pattern to handle different game actions, which could make it easier to add new actions in the future.
Extract common logic (like error handling, transaction submission, and event parsing) into separate utility functions to reduce code duplication.
Consider using TypeScript's discriminated unions for better type safety when handling different types of events.
Use async/await consistently throughout the code for better readability and error handling.
Example of a potential refactor for the
spawnaction:async function handleSpawn(formData: FormData, spawnCalls: Call[]) { startLoading("Create", "Spawning Adventurer", "adventurersByOwnerQuery", undefined); try { const tx = await submitTransaction(spawnCalls); addTransaction(tx, `Spawn ${formData.name}`); const receipt = await waitForTransaction(tx); const events = await parseSpawnEvents(receipt); updateStateFromSpawnEvents(events); stopLoading(`You have spawned ${formData.name}!`, false, "Create"); setScreen("play"); !onKatana && getEthBalance(); } catch (e) { console.log(e); stopLoading(e, true); } }This refactoring would make the code more modular, easier to test, and simpler to maintain and extend in the future.
Line range hint
433-1235: Enhance error handling and loggingThe current error handling in the file uses try-catch blocks and logs errors to the console. While this captures errors, it could be more informative and provide better guidance for debugging and user feedback.
Consider the following improvements to error handling:
Create custom error types for different categories of errors (e.g., NetworkError, ContractError, UserInputError).
Implement a more robust error logging system that includes additional context, such as the function name, input parameters, and stack trace.
Provide more descriptive error messages to the user, possibly with suggestions on how to resolve the issue.
Consider implementing a global error boundary to catch and handle unexpected errors.
For critical errors, consider implementing an error reporting mechanism that sends error details to your backend for monitoring and analysis.
Example of improved error handling:
class GameActionError extends Error { constructor(message: string, public actionType: string, public context: any) { super(message); this.name = 'GameActionError'; } } async function executeGameAction(actionType: string, actionFn: () => Promise<void>) { try { await actionFn(); } catch (error) { console.error(`Error in ${actionType}:`, error); if (error instanceof GameActionError) { stopLoading(`${error.message}. Please try again or contact support.`, true); } else { stopLoading('An unexpected error occurred. Please try again later.', true); } // Optionally, send error details to your error reporting service reportErrorToService(error, actionType); } } // Usage await executeGameAction('spawn', async () => { // spawn logic here if (someErrorCondition) { throw new GameActionError('Failed to spawn adventurer', 'spawn', { formData }); } });This approach provides more structured and informative error handling, improving both the developer experience for debugging and the user experience when errors occur.
Line range hint
1-1235: Replaceanytypes with more specific typesThe file uses the
anytype in several places, which reduces type safety and can lead to potential runtime errors. TypeScript's type system is most effective when specific types are used.Consider replacing
anytypes with more specific types throughout the file. Here are some examples:
In function parameters:
setData: (queryKey: QueryKey, data: any, attribute?: string, index?: number) => voidCould be changed to something like:
setData: <T>(queryKey: QueryKey, data: T, attribute?: keyof T, index?: number) => voidIn event handling:
const events = await parseEvents(receipt as InvokeTransactionReceiptResponse, undefined, beastsContract.address, "StartGame")Consider creating an
Eventinterface or type that describes the structure of your events.In the
handleEquipfunction:function handleEquip(events: any[], ...)Create an
EquipEventinterface that describes the structure of equip events.In the
stopLoadingfunction:stopLoading: (notificationData: any, error?: boolean | undefined, type?: string) => voidCreate a
NotificationDatatype that describes the structure of your notification data.By replacing
anywith more specific types, you'll get better type checking, improved IDE support, and catch potential errors at compile-time rather than runtime. This will lead to more robust and maintainable code.
Line range hint
1-1235: Replace magic numbers and strings with named constantsThe file contains several instances of magic numbers and strings. These can make the code less readable and harder to maintain, especially when the same values are used in multiple places.
Consider replacing magic numbers and strings with named constants. This will improve code readability and make it easier to update these values if needed. Here are some examples:
Replace magic strings for event names:
const EVENT_NAMES = { START_GAME: 'StartGame', AMBUSHED_BY_BEAST: 'AmbushedByBeast', DISCOVERED_HEALTH: 'DiscoveredHealth', // ... other event names } as const;Replace magic numbers:
const APPROVAL_PERCENTAGE = 100; // or 110 if you decide to keep the buffer const ENTROPY_ZERO = BigInt(0); const MAX_ITEM_LEVEL = 20;Replace magic strings for function names:
const FUNCTION_NAMES = { APPROVE: 'approve', TRANSFER: 'transfer', NEW_GAME: 'new_game', // ... other function names } as const;Replace magic strings for screen names:
const SCREEN_NAMES = { PLAY: 'play', START: 'start', UPGRADE: 'upgrade', // ... other screen names } as const;Using these constants instead of magic numbers and strings will make your code more self-documenting and easier to maintain. It also reduces the risk of typos when using these values in multiple places.
Line range hint
1-1235: Summary of review and suggested next stepsThis file contains critical game logic for handling various actions in the game. While it's functional, there are several areas where improvements could be made to enhance maintainability, type safety, and readability.
Key points from the review:
The main change reduces the LORDS token approval amount from 110% to 100% of the cost to play. This change should be carefully considered and documented.
The
createSyscallsfunction is large and complex, handling multiple game actions. Consider refactoring this into smaller, more focused functions or using design patterns to improve modularity.Error handling could be enhanced to provide more context and better user feedback.
The use of
anytypes in several places reduces type safety. Replace these with more specific types where possible.Magic numbers and strings are used throughout the file. Consider replacing these with named constants for improved readability and maintainability.
Suggested next steps:
Review the LORDS token approval change and ensure it doesn't introduce any risks. Add a comment explaining the rationale for this change.
Start refactoring the
createSyscallsfunction, breaking it down into smaller, more manageable pieces. Consider using a command pattern or strategy pattern for different game actions.Implement a more robust error handling system, including custom error types and more informative error messages.
Conduct a type safety pass through the file, replacing
anytypes with more specific types.Create a constants file or object to house all magic numbers and strings, then replace their usage throughout this file.
After making these changes, consider adding or updating unit tests to ensure all functionality still works as expected.
These improvements will lead to a more maintainable, robust, and developer-friendly codebase. Prioritize these tasks based on your team's current needs and development schedule.
ui/src/app/components/start/Spawn.tsx (1)
132-144: RenamealiveAdventurersVariablesfor clarityThe variable
aliveAdventurersVariablesis used with thegetDeadAdventurersByXPPaginatedquery, which fetches dead adventurers. This naming might cause confusion. Consider renaming the variable todeadAdventurersVariablesto accurately reflect its purpose.Apply this change:
- const aliveAdventurersVariables = useMemo(() => { + const deadAdventurersVariables = useMemo(() => { return { skip: 0, }; }, []); const adventurersByXPdata = useCustomQuery( network, "adventurersByXPQuery", getDeadAdventurersByXPPaginated, - aliveAdventurersVariables + deadAdventurersVariables );
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (13)
ui/public/icons/1st.pngis excluded by!**/*.pngui/public/icons/2nd.pngis excluded by!**/*.pngui/public/icons/3rd.pngis excluded by!**/*.pngui/public/icons/client.svgis excluded by!**/*.svgui/public/icons/creator.svgis excluded by!**/*.svgui/public/icons/eth-3.svgis excluded by!**/*.svgui/public/icons/profile.svgis excluded by!**/*.svgui/public/insert-golden-token-hover.pngis excluded by!**/*.pngui/public/insert-golden-token.pngis excluded by!**/*.pngui/public/insert-lords-hover.pngis excluded by!**/*.pngui/public/insert-lords.pngis excluded by!**/*.pngui/public/music/ui/arcade-machine.mp3is excluded by!**/*.mp3ui/public/music/ui/coin.mp3is excluded by!**/*.mp3
Files selected for processing (11)
- ui/src/app/components/adventurer/NftCard.tsx (1 hunks)
- ui/src/app/components/start/PaymentDetails.tsx (1 hunks)
- ui/src/app/components/start/Spawn.tsx (3 hunks)
- ui/src/app/globals.css (1 hunks)
- ui/src/app/hooks/useMusic.tsx (1 hunks)
- ui/src/app/hooks/useUiSound.tsx (1 hunks)
- ui/src/app/lib/constants.ts (1 hunks)
- ui/src/app/lib/networkConfig.ts (4 hunks)
- ui/src/app/lib/utils/syscalls.ts (1 hunks)
- ui/src/app/page.tsx (0 hunks)
- ui/tailwind.config.js (1 hunks)
Files not reviewed due to no reviewable changes (1)
- ui/src/app/page.tsx
Additional comments not posted (12)
ui/src/app/hooks/useUiSound.tsx (3)
12-13: New sound effects added. Please clarify their relation to the PR objectives.The addition of "coin" and "spawn" sound effects enhances the game's audio experience. However, could you please elaborate on how these new sounds relate to the payment page enhancement mentioned in the PR objectives?
27-27: Default volume constant added.The introduction of
defaultVolumeis a good practice. It provides a consistent fallback for any sound effects not explicitly defined in thevolumeMap.
Line range hint
1-37: Please clarify the connection between sound changes and payment page improvements.The changes in this file enhance the sound management system by adding new effects and improving volume control. However, the connection between these audio improvements and the stated PR objective of enhancing the payment page is not immediately clear. Could you please elaborate on how these sound changes contribute to the "Payment QOL" improvements mentioned in the PR description?
ui/src/app/hooks/useMusic.tsx (1)
17-19: LGTM! State initialization updated correctly.The
musicstate initialization has been properly updated to reflect the new structure of themusicSelectorobject. This change ensures type safety and consistency throughout the component.ui/src/app/lib/constants.ts (1)
123-129: LGTM! Consider adding a descriptive comment.The new
payoutsconstant is well-structured and aligns with the PR objective of providing clearer payment information. The values are precise and sum up to 100%, which is correct for a payout distribution.Consider adding a descriptive comment above the constant to explain its purpose and usage. For example:
// Defines the payout distribution percentages for various stakeholders in the Adventurer NFT minting process export const payouts = { // ... (existing code) };Please confirm that these payout percentages are correct and have been approved by the relevant stakeholders.
ui/src/app/lib/networkConfig.ts (3)
26-27: LGTM: Addition of adventurerViewer for Sepolia networkThe addition of the
adventurerViewerproperty for the Sepolia test network is consistent with the PR objectives. It provides a link to view Adventurer NFTs, which enhances the user experience by offering more information about the minting process.
Line range hint
1-126: Overall feedback on networkConfig.ts changesThe changes made to
networkConfig.tsare generally good and align well with the PR objectives:
- The addition of
adventurerViewerproperties across all environments enhances the ability to view Adventurer NFTs, which improves the user experience during the minting process.- The update to
goldenTokenMintUrlin the mainnet configuration provides more specific information, which contributes to clearer payment information.However, there are a few points to consider for further improvement:
- The
adventurerViewerURL is identical across all environments. Consider using environment-specific URLs for development and testing environments (katana and localKatana) to facilitate easier testing and development.- Adding comments to explain the significance of the collection IDs used in the URLs would improve code maintainability.
These changes contribute positively to the "Payment QOL" improvements outlined in the PR objectives. With the suggested refinements, the code will be even more robust and developer-friendly.
86-87: LGTM with a question: Addition of adventurerViewer for Katana environmentThe addition of the
adventurerViewerproperty in the Katana configuration is consistent with the changes in other environments, which is good for maintaining parity across different network setups.However, I noticed that the URL for
adventurerVieweris identical across all environments (sepolia, mainnet, and now katana). Could you confirm if this is intentional? If Katana is a development or testing environment, it might be beneficial to have a separate viewer URL for testing purposes.To verify the usage of this URL across the codebase, you can run the following command:
This will help us understand if the URL is used consistently and if there are any environment-specific implementations we should be aware of.
Verification successful
Confirmed: adventurerViewer URL Consistency Across Environments
I have verified that the
adventurerViewerURL is consistently used across all environments (Katana, Sepolia, Mainnet) withinui/src/app/lib/networkConfig.ts. There are no discrepancies found, indicating that the same viewer URL is intended for all environments.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for uses of the adventurerViewer URL rg "https://market.realms.world/collection/0x018108b32cea514a78ef1b0e4a0753e855cdf620bc0565202c02456f618c4dc4" --type tsLength of output: 708
ui/src/app/globals.css (1)
Line range hint
1-370: Summary: Change aligns well with PR objectivesThe addition of the
.no-text-shadowclass is a small but valuable change that supports the PR's goal of enhancing the payment page. By allowing selective removal of text shadows, it can improve readability of critical information, potentially including payment details and Adventurer NFT minting information.This change is non-intrusive and provides flexibility in styling without affecting the overall design aesthetic. It's a good example of a targeted improvement that can enhance user experience in specific areas of the application.
ui/src/app/components/start/PaymentDetails.tsx (2)
1-9: LGTM: Imports and interface declaration look good.The imports are appropriate for the component's requirements, and the
PaymentDetailsPropsinterface is well-defined and exported for potential reuse.
1-161: Summary: Well-implemented component aligning with PR objectives.This new
PaymentDetailscomponent successfully enhances the payment page by incorporating additional details related to the Adventurer NFT minting process and providing clearer payment information. The implementation is solid, using modern React practices and providing a responsive design.The component effectively displays game prices, payouts for top adventurers, and earnings for client providers and creators, which aligns well with the PR's goal of improving user experience and providing comprehensive transaction information.
While the implementation is generally good, consider the suggested refactors to improve maintainability (extracting hardcoded values), accessibility (adding ARIA attributes), and reducing code duplication. These changes will further enhance the component's quality and user experience.
Overall, this addition successfully contributes to the "Payment QOL" improvements outlined in the PR objectives.
ui/src/app/components/adventurer/NftCard.tsx (1)
1-7: LGTM: Well-structured component declarationThe component interface and function declaration are well-defined using TypeScript, which enhances type safety and code clarity.
| <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 600 900"> | ||
| <style>{`text{text-transform: uppercase;font-family: Courier, monospace;fill: #3DEC00;}g{fill: #3DEC00;}`}</style> | ||
| <rect | ||
| x="0.5" | ||
| y="0.5" | ||
| width="599" | ||
| height="899" | ||
| rx="27.5" | ||
| fill="black" | ||
| stroke="#3DEC00" | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider extracting styles and using dynamic dimensions
While the SVG structure is well-formed, consider the following improvements:
- Extract the inline CSS to a separate stylesheet for better maintainability.
- Use dynamic values for the viewBox and rectangle dimensions to enhance flexibility.
Example:
const NftCard = ({ name, weapon, width = 600, height = 900 }: NftCardProps & { width?: number, height?: number }) => {
return (
<svg xmlns="http://www.w3.org/2000/svg" viewBox={`0 0 ${width} ${height}`}>
{/* ... */}
<rect
x="0.5"
y="0.5"
width={width - 1}
height={height - 1}
rx={Math.min(width, height) * 0.03}
fill="black"
stroke="#3DEC00"
/>
{/* ... */}
</svg>
);
};| </text> | ||
| <text | ||
| x="30" | ||
| y="150" | ||
| font-size="20" | ||
| text-anchor="left" | ||
| dominant-baseline="middle" | ||
| > | ||
| XP: 0 | ||
| </text> | ||
| <text | ||
| x="300" | ||
| y="150" | ||
| font-size="20" | ||
| text-anchor="end" | ||
| dominant-baseline="middle" | ||
| > | ||
| LVL: 1 | ||
| </text> | ||
| <text | ||
| x="570" | ||
| y="58" | ||
| font-size="20" | ||
| text-anchor="end" | ||
| dominant-baseline="right" | ||
| > | ||
| 100 / 100 HP | ||
| </text> | ||
| <text | ||
| x="570" | ||
| y="93" | ||
| font-size="20" | ||
| text-anchor="end" | ||
| dominant-baseline="right" | ||
| > | ||
| 25 GOLD | ||
| </text> | ||
| <text | ||
| x="570" | ||
| y="128" | ||
| font-size="20" | ||
| text-anchor="end" | ||
| dominant-baseline="right" | ||
| > | ||
| ? STR | ||
| </text> | ||
| <text | ||
| x="570" | ||
| y="163" | ||
| font-size="20" | ||
| text-anchor="end" | ||
| dominant-baseline="right" | ||
| > | ||
| ? DEX | ||
| </text> | ||
| <text | ||
| x="570" | ||
| y="198" | ||
| font-size="20" | ||
| text-anchor="end" | ||
| dominant-baseline="right" | ||
| > | ||
| ? INT | ||
| </text> | ||
| <text | ||
| x="570" | ||
| y="233" | ||
| font-size="20" | ||
| text-anchor="end" | ||
| dominant-baseline="right" | ||
| > | ||
| ? VIT | ||
| </text> | ||
| <text | ||
| x="570" | ||
| y="268" | ||
| font-size="20" | ||
| text-anchor="end" | ||
| dominant-baseline="right" | ||
| > | ||
| ? WIS | ||
| </text> | ||
| <text | ||
| x="570" | ||
| y="303" | ||
| font-size="20" | ||
| text-anchor="end" | ||
| dominant-baseline="right" | ||
| > | ||
| ? CHA | ||
| </text> | ||
| <text | ||
| x="570" | ||
| y="338" | ||
| font-size="20" | ||
| text-anchor="end" | ||
| dominant-baseline="right" | ||
| > | ||
| 2 LUCK | ||
| </text> |
There was a problem hiding this comment.
Enhance NFT details rendering with dynamic data
The current implementation has several areas for improvement:
- Replace hardcoded values with dynamic props to increase reusability.
- Implement proper rendering for all attributes instead of using "?".
- Consider using a loop to render attributes for better maintainability.
Example refactor:
interface NftCardProps {
name: string;
weapon: string;
level: number;
xp: number;
hp: { current: number; max: number };
gold: number;
attributes: Record<string, number>;
}
// Inside the component
const attributeOrder = ['STR', 'DEX', 'INT', 'VIT', 'WIS', 'CHA', 'LUCK'];
{attributeOrder.map((attr, index) => (
<text
key={attr}
x="570"
y={128 + index * 35}
fontSize="20"
textAnchor="end"
dominantBaseline="right"
>
{attributes[attr]} {attr}
</text>
))}| dominant-baseline="middle" | ||
| > | ||
| Bag | ||
| </text> | ||
| <g transform="translate(25,240) scale(1.5)"> | ||
| <path d="M8 4V3H6V2H5V1H3v2H2v2H1v1h2V5h2v2H4v2H3v2H2v2H1v2H0v2h2v-2h1v-2h1v-2h1V9h1V7h2v5h2v-2h1V8h1V6h1V4H8Z" /> | ||
| </g> | ||
| <text | ||
| x="60" | ||
| y="253" | ||
| font-size="16" | ||
| text-anchor="start" | ||
| dominant-baseline="middle" | ||
| > | ||
| G1 {weapon} | ||
| </text> | ||
| <g transform="translate(24,280) scale(1.5)"> | ||
| <path d="M0 8h2V7H0v1Zm3-3V2H2v1H1v2H0v1h4V5H3Zm2-4H4v4h1V1Zm6 0v4h1V1h-1Zm4 4V3h-1V2h-1v3h-1v1h4V5h-1Zm-1 3h2V7h-2v1ZM9 7H7V6H4v1H3v4h4v-1h2v1h4V7h-1V6H9v1Zm1 6v1h1v2h1v-2h1v-2H9v1h1Zm-3-1h2v-1H7v1Zm0 1v-1H3v2h1v2h1v-2h1v-1h1Zm2 0H7v1H6v2h4v-2H9v-1Z" /> | ||
| </g> | ||
| <text | ||
| x="60" | ||
| y="292" | ||
| font-size="16" | ||
| text-anchor="left" | ||
| dominant-baseline="middle" | ||
| > | ||
| None Equipped | ||
| </text> | ||
| <g transform="translate(25,320) scale(1.5)"> | ||
| <path d="M12 2h-1V1h-1V0H6v1H5v1H4v1H3v8h1v1h2V8H5V7H4V5h3v4h2V5h3v2h-1v1h-1v4h2v-1h1V3h-1V2ZM2 2V1H1V0H0v2h1v2h1v1-2h1V2H2Zm13-2v1h-1v1h-1v1h1v2-1h1V2h1V0h-1Z" /> | ||
| </g> | ||
| <text | ||
| x="60" | ||
| y="331" | ||
| font-size="16" | ||
| text-anchor="left" | ||
| dominant-baseline="middle" | ||
| > | ||
| None Equipped | ||
| </text> | ||
| <g transform="translate(25,360) scale(1.5)"> | ||
| <path d="M0 13h2v-1H0v1Zm0-2h3v-1H0v1Zm1-7H0v5h3V8h2V3H1v1Zm0-2h4V0H1v2Zm5 0h1V1h1v1h1V0H6v2Zm8-2h-4v2h4V0Zm0 4V3h-4v5h2v1h3V4h-1Zm-2 7h3v-1h-3v1Zm1 2h2v-1h-2v1ZM6 9h1v1h1V9h1V3H6v6Z" /> | ||
| </g> | ||
| <text | ||
| x="60" | ||
| y="370" | ||
| font-size="16" | ||
| text-anchor="left" | ||
| dominant-baseline="middle" | ||
| > | ||
| None Equipped | ||
| </text> | ||
| <g transform="translate(25,400) scale(1.5)"> | ||
| <path d="M4 1V0H0v2h5V1H4Zm2-1H5v1h1V0Zm0 2H5v1h1V2Zm0 2V3H5v1h1Zm0 2V5H5v1h1Zm0 2V7H5v1h1Zm5 0V7H9v1h2Zm0-2V5H9v1h2Zm0-2V3H9v1h2Zm0-2H9v1h2V2Zm0-2H9v1h2V0ZM8 1V0H7v2h2V1H8Zm0 6h1V6H8V5h1V4H8V3h1-2v5h1V7ZM6 9V8H4V7h1V6H4V5h1V4H4V3h1-5v8h5V9h1Zm5 0h-1V8H7v1H6v2H5v1h6V9ZM0 13h5v-1H0v1Zm11 0v-1H5v1h6Zm1 0h4v-1h-4v1Zm3-3V9h-1V8h-2v1h-1v1h1v2h4v-2h-1Zm-4-2v1-1Z" /> | ||
| </g> | ||
| <text | ||
| x="60" | ||
| y="409" | ||
| font-size="16" | ||
| text-anchor="left" | ||
| dominant-baseline="middle" | ||
| > | ||
| None Equipped | ||
| </text> | ||
| <g transform="translate(27,435) scale(1.5)"> | ||
| <path d="M9 8v1H8v3H4v-1h3V2H6v7H5V1H4v8H3V2H2v8H1V5H0v10h1v2h5v-1h2v-1h1v-2h1V8H9Z" /> | ||
| </g> | ||
| <text | ||
| x="60" | ||
| y="448" | ||
| font-size="16" | ||
| text-anchor="left" | ||
| dominant-baseline="middle" | ||
| > | ||
| None Equipped | ||
| </text> | ||
| <g transform="translate(25,475) scale(1.5)"> | ||
| <path d="M14 8V6h-1V5h-1V4h-1V3h-1V2H8V1H2v1H1v1H0v8h1v1h1v1h4v-1h1v-1H3v-1H2V4h1V3h4v1h2v1h1v1h1v1h1v1h1v1h-2v1h1v1h2v-1h1V8h-1Zm-6 3v1h1v-1H8Zm1 0h2v-1H9v1Zm4 3v-2h-1v2h1Zm-6-2v2h1v-2H7Zm2 4h2v-1H9v1Zm-1-2v1h1v-1H8Zm3 1h1v-1h-1v1Zm0-3h1v-1h-1v1Zm-2 2h2v-2H9v2Z" /> | ||
| </g> | ||
| <text | ||
| x="60" | ||
| y="487" | ||
| font-size="16" | ||
| text-anchor="left" | ||
| dominant-baseline="middle" | ||
| > | ||
| None Equipped | ||
| </text> | ||
| <g transform="translate(25,515) scale(1.5)"> | ||
| <path d="M13 3V2h-1V1h-2v1h1v3h-1v2H9v1H8v1H7v1H6v1H4v1H1v-1H0v2h1v1h1v1h4v-1h2v-1h1v-1h1v-1h1v-1h1V9h1V7h1V3h-1ZM3 9h1V8h1V7h1V6h1V5h1V4h2V2H9V1H8v1H6v1H5v1H4v1H3v1H2v1H1v2H0v1h1v1h2V9Z" /> | ||
| </g> | ||
| <text | ||
| x="60" | ||
| y="526" | ||
| font-size="16" | ||
| text-anchor="left" | ||
| dominant-baseline="middle" | ||
| > | ||
| None Equipped | ||
| </text> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve equipment rendering with dynamic data
The current implementation of equipment rendering can be enhanced:
- Use dynamic data to display actually equipped items.
- Create a reusable component for equipment slots to reduce repetition.
- Consider using an array of equipment data to map over instead of hardcoding each slot.
Example refactor:
interface EquipmentSlot {
name: string;
icon: string; // SVG path
equipped: string | null;
}
const EquipmentSlotComponent = ({ slot, index }: { slot: EquipmentSlot, index: number }) => (
<>
<g transform={`translate(25,${240 + index * 40}) scale(1.5)`}>
<path d={slot.icon} />
</g>
<text
x="60"
y={253 + index * 40}
fontSize="16"
textAnchor="start"
dominantBaseline="middle"
>
{slot.equipped || 'None Equipped'}
</text>
</>
);
// Inside the main component
const equipmentSlots: EquipmentSlot[] = [
{ name: 'Weapon', icon: "M8 4V3H6V2H5V1H3v2H2v2H1v1h2V5h2v2H4v2H3v2H2v2H1v2H0v2h2v-2h1v-2h1v-2h1V9h1V7h2v5h2v-2h1V8h1V6h1V4H8Z", equipped: weapon },
// ... other equipment slots
];
{equipmentSlots.map((slot, index) => (
<EquipmentSlotComponent key={slot.name} slot={slot} index={index} />
))}| </text> | ||
| <text | ||
| x="30" | ||
| y="760" | ||
| font-size="16" | ||
| text-anchor="left" | ||
| dominant-baseline="middle" | ||
| > | ||
| 5. Empty | ||
| </text> | ||
| <text | ||
| x="30" | ||
| y="794" | ||
| font-size="16" | ||
| text-anchor="left" | ||
| dominant-baseline="middle" | ||
| > | ||
| 6. Empty | ||
| </text> | ||
| <text | ||
| x="30" | ||
| y="828" | ||
| font-size="16" | ||
| text-anchor="left" | ||
| dominant-baseline="middle" | ||
| > | ||
| 7. Empty | ||
| </text> | ||
| <text | ||
| x="30" | ||
| y="862" | ||
| font-size="16" | ||
| text-anchor="left" | ||
| dominant-baseline="middle" | ||
| > | ||
| 8. Empty | ||
| </text> | ||
| <text | ||
| x="321" | ||
| y="624" | ||
| font-size="16" | ||
| text-anchor="left" | ||
| dominant-baseline="middle" | ||
| > | ||
| 9. Empty | ||
| </text> | ||
| <text | ||
| x="311" | ||
| y="658" | ||
| font-size="16" | ||
| text-anchor="left" | ||
| dominant-baseline="middle" | ||
| > | ||
| 10. Empty | ||
| </text> | ||
| <text | ||
| x="311" | ||
| y="692" | ||
| font-size="16" | ||
| text-anchor="left" | ||
| dominant-baseline="middle" | ||
| > | ||
| 11. Empty | ||
| </text> | ||
| <text | ||
| x="311" | ||
| y="726" | ||
| font-size="16" | ||
| text-anchor="left" | ||
| dominant-baseline="middle" | ||
| > | ||
| 12. Empty | ||
| </text> | ||
| <text | ||
| x="311" | ||
| y="760" | ||
| font-size="16" | ||
| text-anchor="left" | ||
| dominant-baseline="middle" | ||
| > | ||
| 13. Empty | ||
| </text> | ||
| <text | ||
| x="311" | ||
| y="794" | ||
| font-size="16" | ||
| text-anchor="left" | ||
| dominant-baseline="middle" | ||
| > | ||
| 14. Empty | ||
| </text> | ||
| <text | ||
| x="311" | ||
| y="828" | ||
| font-size="16" | ||
| text-anchor="left" | ||
| dominant-baseline="middle" | ||
| > | ||
| 15. Empty | ||
| </text> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize inventory rendering for flexibility and maintainability
The current implementation of inventory rendering can be significantly improved:
- Use dynamic data to display actual inventory items.
- Create a reusable component for inventory slots to reduce repetition.
- Implement a grid layout for better space utilization and scalability.
Example refactor:
interface InventoryItem {
id: number;
name: string | null;
}
const InventorySlot = ({ item }: { item: InventoryItem }) => (
<text
fontSize="16"
textAnchor="left"
dominantBaseline="middle"
>
{item.id}. {item.name || 'Empty'}
</text>
);
// Inside the main component
const inventoryItems: InventoryItem[] = Array(15).fill(null).map((_, index) => ({ id: index + 1, name: null }));
<g transform="translate(30, 624)">
{inventoryItems.map((item, index) => (
<g key={item.id} transform={`translate(${index % 3 * 180}, ${Math.floor(index / 3) * 34})`}>
<InventorySlot item={item} />
</g>
))}
</g>This approach allows for easy expansion of the inventory and better reflects the actual inventory state.
| @@ -433,7 +433,7 @@ export function createSyscalls({ | |||
| entrypoint: "approve", | |||
| calldata: [ | |||
| gameContract?.address ?? "", | |||
| (costToPlay! * 1.1)!.toString(), | |||
| costToPlay!.toString(), | |||
There was a problem hiding this comment.
Approval amount changed from 110% to 100% of costToPlay
The approval amount for the LORDS token has been changed from (costToPlay * 1.1).toString() to costToPlay!.toString(). This modification reduces the approved amount from 110% of the cost to play to exactly 100%.
While this change simplifies the approval process, it removes the 10% buffer that was previously in place. This could potentially lead to issues if there are any fluctuations in the cost to play or if additional fees are introduced in the future. Consider the following:
- Ensure that the
costToPlayvalue is always accurate and up-to-date. - Verify that there are no scenarios where additional fees might be added on top of the
costToPlayamount. - If the 10% buffer was intentionally added for a specific reason, make sure that reason is no longer valid before removing it.
It's recommended to add a comment explaining why this change was made to provide context for future developers.
|
|
||
| useEffect(() => { | ||
| getUsableGoldenToken(goldenTokens ?? []); |
There was a problem hiding this comment.
Add goldenTokens to the useEffect dependency array
The useEffect hook depends on goldenTokens, but the dependency array is currently empty. To ensure that getUsableGoldenToken is called whenever goldenTokens changes, add goldenTokens to the dependency array.
Apply this change:
useEffect(() => {
getUsableGoldenToken(goldenTokens ?? []);
- }, []);
+ }, [goldenTokens]);Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| getUsableGoldenToken(goldenTokens ?? []); | |
| useEffect(() => { | |
| getUsableGoldenToken(goldenTokens ?? []); | |
| }, [goldenTokens]); |
| onClick={() => { | ||
| if (usableToken !== "0") { | ||
| handlePayment(true); | ||
| } else { | ||
| handlePayment(false); | ||
| } | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify the onClick handler by passing the condition directly
The onClick handler currently uses an if...else statement to decide the argument for handlePayment. This can be simplified by passing the condition usableToken !== "0" directly to the function.
Update the onClick handler as follows:
onClick={() => {
- if (usableToken !== "0") {
- handlePayment(true);
- } else {
- handlePayment(false);
- }
+ handlePayment(usableToken !== "0");
}}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onClick={() => { | |
| if (usableToken !== "0") { | |
| handlePayment(true); | |
| } else { | |
| handlePayment(false); | |
| } | |
| }} | |
| onClick={() => { | |
| handlePayment(usableToken !== "0"); | |
| }} |
| ) : ( | ||
| <span className="flex flex-row gap-1 items-center justify-center"> | ||
| <Lords className="self-center w-24 h-24 fill-current" /> | ||
| <p className="text-6xl no-text-shadow">59</p> |
There was a problem hiding this comment.
Use dynamic game cost instead of hardcoded value
The cost displayed is hardcoded as 59. To ensure consistency and accommodate future changes, use the lordsGameCost variable instead.
Replace the hardcoded value with the dynamic variable:
<p className="text-6xl no-text-shadow">
- 59
+ {lordsGameCost}
</p>Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <p className="text-6xl no-text-shadow">59</p> | |
| <p className="text-6xl no-text-shadow">{lordsGameCost}</p> |
| setPaymentInitiated(true); | ||
| try { | ||
| await spawn( | ||
| formData, | ||
| goldenToken ? usableToken : "0", | ||
| networkConfig[network!].revenueAddresses, | ||
| lordsGameCost | ||
| ); | ||
| await getBalances(); | ||
| setPaymentInitiated(false); | ||
| } catch (error) { | ||
| console.error(error); | ||
| setPaymentInitiated(false); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Eliminate duplicate setPaymentInitiated(false) calls
The setPaymentInitiated(false); statement is present in both the try block and the catch block. This can be simplified by moving it outside the try...catch block to ensure it runs regardless of the outcome, reducing code duplication.
Apply this diff to refactor the code:
try {
await spawn(
formData,
goldenToken ? usableToken : "0",
networkConfig[network!].revenueAddresses,
lordsGameCost
);
await getBalances();
- setPaymentInitiated(false);
} catch (error) {
console.error(error);
- setPaymentInitiated(false);
}
+ setPaymentInitiated(false);Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| setPaymentInitiated(true); | |
| try { | |
| await spawn( | |
| formData, | |
| goldenToken ? usableToken : "0", | |
| networkConfig[network!].revenueAddresses, | |
| lordsGameCost | |
| ); | |
| await getBalances(); | |
| setPaymentInitiated(false); | |
| } catch (error) { | |
| console.error(error); | |
| setPaymentInitiated(false); | |
| } | |
| setPaymentInitiated(true); | |
| try { | |
| await spawn( | |
| formData, | |
| goldenToken ? usableToken : "0", | |
| networkConfig[network!].revenueAddresses, | |
| lordsGameCost | |
| ); | |
| await getBalances(); | |
| } catch (error) { | |
| console.error(error); | |
| } | |
| setPaymentInitiated(false); |
- center payment table
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (1)
ui/src/app/components/start/Spawn.tsx (1)
Line range hint
1-357: Consider refactoring for improved maintainabilityThe
Spawncomponent has grown significantly in complexity with the addition of new features and conditional rendering. While it currently implements the required functionality well, its size and the mix of game-specific logic with UI rendering might make it harder to maintain and extend in the future.Consider refactoring the component into smaller, more focused components. For example:
- Create a separate
PaymentInterfacecomponent to handle the payment UI and logic.- Extract the adventurer preview card into its own component.
- Create a
MobileControlscomponent for mobile-specific UI elements.This refactoring would improve the overall maintainability of the code, make it easier to test individual parts of the UI, and potentially improve performance through more granular rendering control.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- ui/src/app/components/start/Spawn.tsx (3 hunks)
Additional comments not posted (3)
ui/src/app/components/start/Spawn.tsx (3)
1-18: LGTM: New imports and state variables enhance functionalityThe additions of sound-related hooks, new state variables for payment and UI interaction, and query-related variables for fetching adventurer data are well-implemented. These changes support improved user experience and data management within the component.
Also applies to: 50-54, 132-145
153-278: LGTM: Enhanced payment interface with improved user feedbackThe payment interface has been significantly improved with the addition of conditional rendering based on the
paymentInitiatedstate, integration of thePaymentDetailscomponent, and enhanced visual feedback for users. These changes contribute to a more informative and interactive user experience during the payment process.
112-130: 🛠️ Refactor suggestionSimplify error handling in handlePayment function
The
handlePaymentfunction is well-structured, but there's a minor optimization opportunity in the error handling.Consider moving the
setPaymentInitiated(false)call outside the try-catch block to reduce code duplication:const handlePayment = async (goldenToken: boolean) => { spawnPlay(); coinPlay(); resetNotification(); setPaymentInitiated(true); try { await spawn( formData, goldenToken ? usableToken : "0", networkConfig[network!].revenueAddresses, lordsGameCost ); await getBalances(); - setPaymentInitiated(false); } catch (error) { console.error(error); - setPaymentInitiated(false); } + setPaymentInitiated(false); };This change ensures that
setPaymentInitiated(false)is called regardless of whether the payment succeeds or fails, while reducing code duplication.Likely invalid or redundant comment.
| <Image | ||
| src={ | ||
| usableToken !== "0" | ||
| ? isHoveringLords | ||
| ? "/insert-golden-token-hover.png" | ||
| : "/insert-golden-token.png" | ||
| : isHoveringLords | ||
| ? "/insert-lords-hover.png" | ||
| : "/insert-lords.png" | ||
| } | ||
| alt="insert-lords" | ||
| fill | ||
| /> | ||
| </span> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify image source selection logic
The nested ternary operators used for selecting the image source reduce code readability.
Consider extracting this logic into a separate function or variable for improved clarity:
const getImageSource = () => {
if (usableToken !== "0") {
return isHoveringLords ? "/insert-golden-token-hover.png" : "/insert-golden-token.png";
}
return isHoveringLords ? "/insert-lords-hover.png" : "/insert-lords.png";
};
// Then in the JSX:
<Image
src={getImageSource()}
alt="insert-lords"
fill
/>This refactoring improves code readability while maintaining the same functionality.
| {!paymentInitiated && ( | ||
| <div className="absolute sm:hidden bottom-16 left-0 right-0 flex flex-col items-center gap-4 z-10 pb-8"> | ||
| <Button | ||
| size={"xl"} | ||
| disabled={ | ||
| !formFilled || | ||
| !account || | ||
| isWrongNetwork || | ||
| loading || | ||
| estimatingFee | ||
| } | ||
| onClick={() => handleSubmitLords()} | ||
| className="relative" | ||
| size={"sm"} | ||
| variant={"default"} | ||
| onClick={() => setShowPaymentDetails(!showPaymentDetails)} | ||
| > | ||
| <div className="flex flex-row items-center gap-1 w-full h-full"> | ||
| {formFilled ? "Play as Guest" : "Fill details"} | ||
| </div> | ||
| {showPaymentDetails ? "Hide Payment Details" : "Payment Details"} | ||
| </Button> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve button accessibility for mobile view
The addition of a toggle button for showing/hiding payment details on mobile is a good improvement for user experience. However, the button text could be more descriptive for better accessibility.
Consider updating the button text to be more descriptive:
<Button
size={"sm"}
variant={"default"}
onClick={() => setShowPaymentDetails(!showPaymentDetails)}
>
- {showPaymentDetails ? "Hide Payment Details" : "Payment Details"}
+ {showPaymentDetails ? "Hide Payment Details" : "Show Payment Details"}
</Button>This change makes the button's purpose clearer, especially for screen reader users.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {!paymentInitiated && ( | |
| <div className="absolute sm:hidden bottom-16 left-0 right-0 flex flex-col items-center gap-4 z-10 pb-8"> | |
| <Button | |
| size={"xl"} | |
| disabled={ | |
| !formFilled || | |
| !account || | |
| isWrongNetwork || | |
| loading || | |
| estimatingFee | |
| } | |
| onClick={() => handleSubmitLords()} | |
| className="relative" | |
| size={"sm"} | |
| variant={"default"} | |
| onClick={() => setShowPaymentDetails(!showPaymentDetails)} | |
| > | |
| <div className="flex flex-row items-center gap-1 w-full h-full"> | |
| {formFilled ? "Play as Guest" : "Fill details"} | |
| </div> | |
| {showPaymentDetails ? "Hide Payment Details" : "Payment Details"} | |
| </Button> | |
| </div> | |
| )} | |
| {!paymentInitiated && ( | |
| <div className="absolute sm:hidden bottom-16 left-0 right-0 flex flex-col items-center gap-4 z-10 pb-8"> | |
| <Button | |
| size={"sm"} | |
| variant={"default"} | |
| onClick={() => setShowPaymentDetails(!showPaymentDetails)} | |
| > | |
| {showPaymentDetails ? "Hide Payment Details" : "Show Payment Details"} | |
| </Button> | |
| </div> | |
| )} |
Improve the payment page to show details of the Adventurer NFT minting and the payment details
Summary by CodeRabbit
Release Notes
New Features
Spawncomponent for improved payment handling and user interactions, including a dynamic payment interface.Improvements
Styling
.no-text-shadowadded to remove text shadows.Configuration Updates
adventurerViewerURLs for multiple environments and updated minting URLs.