Loaf config - Githubissues

ponderingdemocritus commented 1 month ago

PR Type

enhancement, bug fix

Description

Updated modal container and market modal styling for better user experience.
Improved market order panel labels and layout for clarity.
Enhanced headline component styling and updated colors.
Refactored top middle navigation component for better performance and added memoization.
Refactored configuration functions to batch calls and improved logging.
Updated provider methods and type definitions to support batched configuration calls.
Updated contract addresses and class hashes in manifest files and removed outdated model definitions.

Changes walkthrough 📝

Relevant files

Enhancement

8 files

ModalContainer.tsx `Update modal container styling.` client/src/ui/components/ModalContainer.tsx Changed modal container background to transparent and added padding.	+1/-1
MarketModal.tsx `Enhance market modal styling and layout.` client/src/ui/components/trading/MarketModal.tsx Updated modal styling and added a title for the market. Adjusted text sizes for better readability.	+5/-2
MarketOrderPanel.tsx `Improve market order panel labels and layout.` client/src/ui/components/trading/MarketOrderPanel.tsx Renamed button labels for clarity. Improved layout and text formatting.	+13/-10
Headline.tsx `Enhance headline component styling.` client/src/ui/elements/Headline.tsx Enhanced headline component styling. Updated SVG and background colors.	+14/-9
TopMiddleNavigation.tsx `Refactor top middle navigation component.` client/src/ui/modules/navigation/TopMiddleNavigation.tsx Refactored navigation component for better performance. Added memoization for structures and hex view.	+15/-17
index.ts `Batch configuration calls and improve logging.` sdk/packages/eternum/src/config/index.ts Refactored configuration functions to batch calls. Improved logging for configuration processes.	+24/-11
index.ts `Support batched configuration calls in provider.` sdk/packages/eternum/src/provider/index.ts - Updated provider methods to handle batched configuration calls.	+27/-12
provider.ts `Update type definitions for batched configuration calls.` sdk/packages/eternum/src/types/provider.ts - Updated type definitions to support batched configuration calls.	+7/-7

Configuration changes

2 files

manifest.json `Update contract addresses and clean up manifest.` contracts/manifests/dev/manifest.json Updated contract addresses and class hashes. Removed outdated model definitions.	+9/-236
manifest.toml `Update contract addresses and clean up manifest.` contracts/manifests/dev/manifest.toml Updated contract addresses and class hashes. Removed outdated model definitions.	+4/-21

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

vercel[bot] commented 1 month ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
eternum	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 13, 2024 8:15am

github-actions[bot] commented 1 month ago

PR Reviewer Guide 🔍

⏱️ Estimated effort to review [1-5]	4
🧪 Relevant tests	No
🔒 Security concerns	No
⚡ Key issues to review	Possible Bug: The changes in `MarketModal.tsx` and `MarketOrderPanel.tsx` include updates to the UI text and layout. Ensure that these changes do not affect the functionality of the market modal, especially the new headers and labels which might not have been tested thoroughly.
	Refactoring Concern: In `TopMiddleNavigation.tsx`, the use of `useMemo` for sorting structures and checking if the location includes `/hex` should be carefully reviewed to ensure that it does not introduce unnecessary re-renders or affect performance negatively.
	Batch Processing: The refactoring in `sdk/packages/eternum/src/config/index.ts` and `sdk/packages/eternum/src/provider/index.ts` to batch configuration calls needs a thorough review to ensure that the batch processing is handled correctly and efficiently without missing any data during the transaction.

github-actions[bot] commented 1 month ago

PR Code Suggestions ✨

Category	Suggestion	Score
Possible issue	Add a check to ensure `calldataArray` is not empty before sending a transaction ___ Consider checking the length of `calldataArray` before attempting to send the transaction. If `calldataArray` is empty, the transaction should not be attempted as it will likely fail or be rejected, leading to unnecessary network fees or errors. [sdk/packages/eternum/src/config/index.ts [46]](https://github.com/BibliothecaDAO/eternum/pull/918/files#diff-6aecc823aea65bfad1461922fd49c884b2c705a136856aecefc623f166212043R46-R46) ```diff -const tx = await provider.set_production_config({ signer: account, calls: calldataArray }); +if (calldataArray.length > 0) { + const tx = await provider.set_production_config({ signer: account, calls: calldataArray }); + console.log(`Configuring resource production ${tx.statusReceipt}...`); +} else { + console.log("No valid data to configure resource production."); +} ``` Suggestion importance[1-10]: 9 Why: This suggestion is crucial as it prevents unnecessary transactions that could fail or incur network fees if `calldataArray` is empty. It improves the robustness and efficiency of the code.	9
	Ensure button text accurately reflects the user's intended action ___ Ensure that the dynamic text for the button reflects the action accurately, especially in different states (buy/sell). [client/src/ui/components/trading/MarketOrderPanel.tsx [434]](https://github.com/BibliothecaDAO/eternum/pull/918/files#diff-00bad72a59745d3d904730035fcdeaddda30f53c9212d94f35f277ef202bd0bdR434-R434) ```diff -{isBuy ? "Buy" : `Sell `} {resource.toLocaleString()} {findResourceById(resourceId)?.trait} +{isBuy ? "Buy" : "Sell"} {resource.toLocaleString()} {findResourceById(resourceId)?.trait \|\| "Resource"} ``` Suggestion importance[1-10]: 8 Why: Ensuring the button text accurately reflects the user's intended action is important for user experience and clarity, especially in different states (buy/sell). This suggestion addresses a potential issue effectively.	8
	Verify and adjust the opacity of the background color if necessary ___ Ensure that the removal of the opacity style from 'bg-brown' is intentional. If the change affects visibility or design negatively, consider adding it back or adjusting the opacity level. [client/src/ui/components/trading/MarketModal.tsx [45]](https://github.com/BibliothecaDAO/eternum/pull/918/files#diff-60c160393d9cf2b5da3291c081dd0d823b49d1cb1dcdda811544686c0ddd347eR45-R45) ```diff - + ``` Suggestion importance[1-10]: 6 Why: The suggestion to ensure the removal of the opacity style is intentional is valid, as it could affect the design. However, it is not a critical issue unless it negatively impacts visibility or design.	6
Possible bug	Ensure type consistency by converting `entityId` to a string ___ To ensure that the `entityId` parameter is always a string, consider explicitly converting it to a string before comparing it in the `goToHexView` function. This can prevent potential type mismatches and bugs. [client/src/ui/modules/navigation/TopMiddleNavigation.tsx [63]](https://github.com/BibliothecaDAO/eternum/pull/918/files#diff-98461a600ecf71fa8776ed334c66d832ea531c99b49221910e7aea02abe61b95R63-R63) ```diff const goToHexView = (entityId: any) => { - const structure = structures.find((structure) => structure.entity_id?.toString() === entityId); + const structure = structures.find((structure) => structure.entity_id?.toString() === entityId.toString()); ``` Suggestion importance[1-10]: 9 Why: Converting `entityId` to a string before comparison ensures type consistency and prevents potential bugs due to type mismatches. This is a crucial improvement for preventing runtime errors.	9
Enhancement	Add error handling for the asynchronous operation to improve robustness ___ It is recommended to handle potential exceptions that might be thrown by the asynchronous call to `set_production_config`. This can be done using a try-catch block to ensure that any errors are caught and handled gracefully. [sdk/packages/eternum/src/config/index.ts [46]](https://github.com/BibliothecaDAO/eternum/pull/918/files#diff-6aecc823aea65bfad1461922fd49c884b2c705a136856aecefc623f166212043R46-R46) ```diff -const tx = await provider.set_production_config({ signer: account, calls: calldataArray }); +try { + const tx = await provider.set_production_config({ signer: account, calls: calldataArray }); + console.log(`Configuring resource production ${tx.statusReceipt}...`); +} catch (error) { + console.error("Failed to configure resource production:", error); +} ``` Suggestion importance[1-10]: 8 Why: Adding error handling is a good practice to ensure that any potential issues during the asynchronous operation are caught and handled gracefully, improving the overall robustness of the code.	8
	Add background cover style to maintain design consistency ___ Consider adding a background cover style to maintain consistency with the previous design, unless the removal was intentional for a specific design requirement. [client/src/ui/components/ModalContainer.tsx [15]](https://github.com/BibliothecaDAO/eternum/pull/918/files#diff-a1513644e65e84da86182ce65ad29af34c4ece158341ed1f463f32ecde609985R15-R15) ```diff -? "w-full h-full bg-transparent p-4" +? "w-full h-full bg-transparent bg-cover p-4" ``` Suggestion importance[1-10]: 7 Why: The suggestion to add a background cover style is valid for maintaining design consistency. However, it is not crucial unless the removal was unintentional or affects the design negatively.	7
	Improve type safety by specifying a more precise type for `className` ___ Consider using a more specific type for `className` in the `Headline` component to ensure it only receives valid CSS class names. This can help prevent runtime errors and improve maintainability. [client/src/ui/elements/Headline.tsx [13]](https://github.com/BibliothecaDAO/eternum/pull/918/files#diff-58c6b068f95da4b941ade1f23a87a33ab911b54cbbdfa6e0942fa4c4b4778168R13-R13) ```diff -export const Headline = ({ children, className }: HeadlineProps) => ( +export const Headline = ({ children, className }: HeadlineProps & { className: string }) => ( ``` Suggestion importance[1-10]: 7 Why: Specifying a more precise type for `className` can help prevent runtime errors and improve maintainability. However, this change is not critical and mainly enhances type safety.	7
	Use more descriptive button texts for clarity ___ Consider using a more descriptive text than 'Trade' and 'Receive' to enhance user understanding, especially if these buttons have significant actions. [client/src/ui/components/trading/MarketOrderPanel.tsx [349-390]](https://github.com/BibliothecaDAO/eternum/pull/918/files#diff-00bad72a59745d3d904730035fcdeaddda30f53c9212d94f35f277ef202bd0bdR349-R390) ```diff -Trade -Receive +Execute Trade +Confirm Receipt ``` Suggestion importance[1-10]: 5 Why: Using more descriptive button texts can enhance user understanding, but the current terms 'Trade' and 'Receive' are not incorrect. This suggestion improves clarity but is not essential.	5
Maintainability	Extract SVG elements into separate components for better readability and maintainability ___ To enhance code readability and maintainability, consider extracting the SVG elements into separate, reusable React components. This will make the `Headline` component cleaner and easier to manage. [client/src/ui/elements/Headline.tsx [21-25]](https://github.com/BibliothecaDAO/eternum/pull/918/files#diff-58c6b068f95da4b941ade1f23a87a33ab911b54cbbdfa6e0942fa4c4b4778168R21-R25) ```diff -Suggestion importance[1-10]: 8 Why: Extracting SVG elements into separate components significantly improves code readability and maintainability. This is a good practice for managing complex JSX structures.	8
Performance	Memoize `hexPosition` to enhance performance ___ To avoid potential performance issues, consider memoizing the `hexPosition` value if it is derived from a complex computation or external data source. [client/src/ui/modules/navigation/TopMiddleNavigation.tsx [42]](https://github.com/BibliothecaDAO/eternum/pull/918/files#diff-98461a600ecf71fa8776ed334c66d832ea531c99b49221910e7aea02abe61b95R42-R42) ```diff -const { hexPosition } = useQuery(); +const hexPosition = useMemo(() => useQuery().hexPosition, [useQuery]); ``` Suggestion importance[1-10]: 6 Why: Memoizing `hexPosition` can enhance performance if it is derived from a complex computation or external data source. However, the suggestion does not provide enough context to determine if `useQuery` is expensive enough to warrant memoization.	6

Category	Suggestion	Score
Possible issue	Add a check to ensure `calldataArray` is not empty before sending a transaction ___ Consider checking the length of `calldataArray` before attempting to send the transaction. If `calldataArray` is empty, the transaction should not be attempted as it will likely fail or be rejected, leading to unnecessary network fees or errors. [sdk/packages/eternum/src/config/index.ts [46]](https://github.com/BibliothecaDAO/eternum/pull/918/files#diff-6aecc823aea65bfad1461922fd49c884b2c705a136856aecefc623f166212043R46-R46) ```diff -const tx = await provider.set_production_config({ signer: account, calls: calldataArray }); +if (calldataArray.length > 0) { + const tx = await provider.set_production_config({ signer: account, calls: calldataArray }); + console.log(`Configuring resource production ${tx.statusReceipt}...`); +} else { + console.log("No valid data to configure resource production."); +} ``` Suggestion importance[1-10]: 9 Why: This suggestion is crucial as it prevents unnecessary transactions that could fail or incur network fees if `calldataArray` is empty. It improves the robustness and efficiency of the code.	9
	Ensure button text accurately reflects the user's intended action ___ Ensure that the dynamic text for the button reflects the action accurately, especially in different states (buy/sell). [client/src/ui/components/trading/MarketOrderPanel.tsx [434]](https://github.com/BibliothecaDAO/eternum/pull/918/files#diff-00bad72a59745d3d904730035fcdeaddda30f53c9212d94f35f277ef202bd0bdR434-R434) ```diff -{isBuy ? "Buy" : `Sell `} {resource.toLocaleString()} {findResourceById(resourceId)?.trait} +{isBuy ? "Buy" : "Sell"} {resource.toLocaleString()} {findResourceById(resourceId)?.trait \|\| "Resource"} ``` Suggestion importance[1-10]: 8 Why: Ensuring the button text accurately reflects the user's intended action is important for user experience and clarity, especially in different states (buy/sell). This suggestion addresses a potential issue effectively.	8
	Verify and adjust the opacity of the background color if necessary ___ Ensure that the removal of the opacity style from 'bg-brown' is intentional. If the change affects visibility or design negatively, consider adding it back or adjusting the opacity level. [client/src/ui/components/trading/MarketModal.tsx [45]](https://github.com/BibliothecaDAO/eternum/pull/918/files#diff-60c160393d9cf2b5da3291c081dd0d823b49d1cb1dcdda811544686c0ddd347eR45-R45) ```diff - + ``` Suggestion importance[1-10]: 6 Why: The suggestion to ensure the removal of the opacity style is intentional is valid, as it could affect the design. However, it is not a critical issue unless it negatively impacts visibility or design.	6
Possible bug	Ensure type consistency by converting `entityId` to a string ___ To ensure that the `entityId` parameter is always a string, consider explicitly converting it to a string before comparing it in the `goToHexView` function. This can prevent potential type mismatches and bugs. [client/src/ui/modules/navigation/TopMiddleNavigation.tsx [63]](https://github.com/BibliothecaDAO/eternum/pull/918/files#diff-98461a600ecf71fa8776ed334c66d832ea531c99b49221910e7aea02abe61b95R63-R63) ```diff const goToHexView = (entityId: any) => { - const structure = structures.find((structure) => structure.entity_id?.toString() === entityId); + const structure = structures.find((structure) => structure.entity_id?.toString() === entityId.toString()); ``` Suggestion importance[1-10]: 9 Why: Converting `entityId` to a string before comparison ensures type consistency and prevents potential bugs due to type mismatches. This is a crucial improvement for preventing runtime errors.	9
Enhancement	Add error handling for the asynchronous operation to improve robustness ___ It is recommended to handle potential exceptions that might be thrown by the asynchronous call to `set_production_config`. This can be done using a try-catch block to ensure that any errors are caught and handled gracefully. [sdk/packages/eternum/src/config/index.ts [46]](https://github.com/BibliothecaDAO/eternum/pull/918/files#diff-6aecc823aea65bfad1461922fd49c884b2c705a136856aecefc623f166212043R46-R46) ```diff -const tx = await provider.set_production_config({ signer: account, calls: calldataArray }); +try { + const tx = await provider.set_production_config({ signer: account, calls: calldataArray }); + console.log(`Configuring resource production ${tx.statusReceipt}...`); +} catch (error) { + console.error("Failed to configure resource production:", error); +} ``` Suggestion importance[1-10]: 8 Why: Adding error handling is a good practice to ensure that any potential issues during the asynchronous operation are caught and handled gracefully, improving the overall robustness of the code.	8
	Add background cover style to maintain design consistency ___ Consider adding a background cover style to maintain consistency with the previous design, unless the removal was intentional for a specific design requirement. [client/src/ui/components/ModalContainer.tsx [15]](https://github.com/BibliothecaDAO/eternum/pull/918/files#diff-a1513644e65e84da86182ce65ad29af34c4ece158341ed1f463f32ecde609985R15-R15) ```diff -? "w-full h-full bg-transparent p-4" +? "w-full h-full bg-transparent bg-cover p-4" ``` Suggestion importance[1-10]: 7 Why: The suggestion to add a background cover style is valid for maintaining design consistency. However, it is not crucial unless the removal was unintentional or affects the design negatively.	7
	Improve type safety by specifying a more precise type for `className` ___ Consider using a more specific type for `className` in the `Headline` component to ensure it only receives valid CSS class names. This can help prevent runtime errors and improve maintainability. [client/src/ui/elements/Headline.tsx [13]](https://github.com/BibliothecaDAO/eternum/pull/918/files#diff-58c6b068f95da4b941ade1f23a87a33ab911b54cbbdfa6e0942fa4c4b4778168R13-R13) ```diff -export const Headline = ({ children, className }: HeadlineProps) => ( +export const Headline = ({ children, className }: HeadlineProps & { className: string }) => ( ``` Suggestion importance[1-10]: 7 Why: Specifying a more precise type for `className` can help prevent runtime errors and improve maintainability. However, this change is not critical and mainly enhances type safety.	7
	Use more descriptive button texts for clarity ___ Consider using a more descriptive text than 'Trade' and 'Receive' to enhance user understanding, especially if these buttons have significant actions. [client/src/ui/components/trading/MarketOrderPanel.tsx [349-390]](https://github.com/BibliothecaDAO/eternum/pull/918/files#diff-00bad72a59745d3d904730035fcdeaddda30f53c9212d94f35f277ef202bd0bdR349-R390) ```diff -Trade -Receive +Execute Trade +Confirm Receipt ``` Suggestion importance[1-10]: 5 Why: Using more descriptive button texts can enhance user understanding, but the current terms 'Trade' and 'Receive' are not incorrect. This suggestion improves clarity but is not essential.	5
Maintainability	Extract SVG elements into separate components for better readability and maintainability ___ To enhance code readability and maintainability, consider extracting the SVG elements into separate, reusable React components. This will make the `Headline` component cleaner and easier to manage. [client/src/ui/elements/Headline.tsx [21-25]](https://github.com/BibliothecaDAO/eternum/pull/918/files#diff-58c6b068f95da4b941ade1f23a87a33ab911b54cbbdfa6e0942fa4c4b4778168R21-R25) ```diff -Suggestion importance[1-10]: 8 Why: Extracting SVG elements into separate components significantly improves code readability and maintainability. This is a good practice for managing complex JSX structures.	8
Performance	Memoize `hexPosition` to enhance performance ___ To avoid potential performance issues, consider memoizing the `hexPosition` value if it is derived from a complex computation or external data source. [client/src/ui/modules/navigation/TopMiddleNavigation.tsx [42]](https://github.com/BibliothecaDAO/eternum/pull/918/files#diff-98461a600ecf71fa8776ed334c66d832ea531c99b49221910e7aea02abe61b95R42-R42) ```diff -const { hexPosition } = useQuery(); +const hexPosition = useMemo(() => useQuery().hexPosition, [useQuery]); ``` Suggestion importance[1-10]: 6 Why: Memoizing `hexPosition` can enhance performance if it is derived from a complex computation or external data source. However, the suggestion does not provide enough context to determine if `useQuery` is expensive enough to warrant memoization.	6

BibliothecaDAO / eternum

Loaf config #918

PR Type

Description

Changes walkthrough 📝

PR Reviewer Guide 🔍

PR Code Suggestions ✨