Skip to content
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

feat: Updates for service form & network dropdown #22

Merged
merged 15 commits into from
Apr 11, 2024

Conversation

mohandast52
Copy link
Collaborator

@mohandast52 mohandast52 commented Apr 10, 2024

Proposed changes

  • Converted a few files to Typescript

Fixes

  • The Component/Agent Update should open the Update hash modal as expected.
Update.hash.mov

NOTE: The Service update button should be visible only to the owner and should redirect to the /update page.

  • Full height of the network dropdown
update
  • Improved the user experience for the Threshold input in the service mint.
threshold.mov

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply

  • Bugfix (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 not work as expected)

@mohandast52 mohandast52 added bug Something isn't working enhancement New feature or request labels Apr 10, 2024
@mohandast52 mohandast52 self-assigned this Apr 10, 2024
Copy link

vercel bot commented Apr 10, 2024

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

Name Status Preview Comments Updated (UTC)
autonolas-registry ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 11, 2024 0:22am
tokenomics ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 11, 2024 0:22am

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Duplicate file

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Duplicate file

@mohandast52 mohandast52 changed the title (WIP) feat: Updates for service form & network dropdown feat: Updates for service form & network dropdown Apr 10, 2024
@oaksprout oaksprout self-requested a review April 10, 2024 16:45
callback(hash);
}
})
.catch(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

not high priority
this .catch renders the try catch block surrounding the code redundant, can remove

apps/autonolas-registry/common-util/hooks/useMetadata.jsx Outdated Show resolved Hide resolved
@@ -11,14 +11,16 @@ export const URL = {
DISCLAIMER: '/disclaimer',
PAGE_NOT_FOUND: '/page-not-found',
NOT_LEGAL: '/not-legal',
};
} as const;
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice, this is super useful, will use myself moving forward

apps/autonolas-registry/common-util/Details/index.tsx Outdated Show resolved Hide resolved
apps/autonolas-registry/common-util/hooks/useMetadata.jsx Outdated Show resolved Hide resolved
apps/autonolas-registry/common-util/hooks/useMetadata.jsx Outdated Show resolved Hide resolved
/**
* validates the threshold based on the no. of slots
*/
const validateThreshold = (
Copy link
Collaborator

Choose a reason for hiding this comment

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

@mohandast52 @truemiller what are your thought on keeping these utils and constants in separate files close to the Component e.g.

ComponentFolder
   |   Component.tsx
   │   utils.ts
   |   hooks.ts
   |   constants.ts
   etc

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Tanya-atatakai @truemiller I believe we shouldn't create separate files for individual components or functions. If a component or a function isn't going to be used by any other component or function, we can consider it a private component or function. It's better to keep these in the same file to provide abstraction. Of course, if a component grows larger, we break it into smaller chunks. However, maintaining a separate file for every component is very cumbersome. WDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

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

utils
	functions
		stringManipulations.js
		stringManipulations.test.js
		ethParsers.js
		ethParsers.test.js
		graphFunctions.js
		graphFunctions.test.js
	constants
		dogs.js
		cats.js
		metaData.js
		graphQueries.js

components
	Layout
		Layout.jsx
		Layout.test.jsx
		Header.jsx
		Header.test.jsx
		Footer.jsx
		Footer.test.jsx

	MagicButton
		MagicButton.jsx
		MagicButton.test.jsx
		
	Table
		Table.jsx
		Table.test.jsx

	DogDisplay
		Dog.jsx
		Dog.test.jsx
		DogTail.jsx
		DogTail.test.jsx

	CatDisplay
		Cat.jsx
		Cat.test.jsx
		CatBowl.jsx
		CatBowl.test.jsx
hooks
	useDog.js
	useDog.test.js
	useCat.js
	useCat.test.js
pages
	index.jsx
	comparison.jsx
	dog.jsx
	cat.jsx
	contact.jsx

this way we know what files to expect in each folder without having to dig or being confused when we find a utils.jsx in a component folder, or some import from another component's folder if the utils or helper functions are reused and not moved to a common folder

we should consider functions and utils as reusable especially if they are exported, and seperate them from components within reason.

if a function or constant value is truly private to a component or hook, we can leave it outside the component definition in the component file, not exported, provided it's not huge. in which case, it probably deserves to be made more succinct, or split into reusable elements. or.. we include it in the component, and use some memoization through useMemo or useCallback

@@ -123,4 +124,36 @@ describe('listAgents/details.jsx', () => {
);
});
});

it('should display update hash button only for owner and open the modal when clicked', async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

kudos for tests!!! 👍

apps/autonolas-registry/util/constants.tsx Outdated Show resolved Hide resolved
Co-authored-by: Atatakai <densatsu@mail.ru>
@mohandast52 mohandast52 merged commit 986b074 into main Apr 11, 2024
5 checks passed
@mohandast52 mohandast52 deleted the mohan/minor-updates branch April 11, 2024 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants