-
Notifications
You must be signed in to change notification settings - Fork 81
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: add contribute tab #3102
feat: add contribute tab #3102
Conversation
meelrossi
commented
May 13, 2024
•
edited
Loading
edited
![image](https://private-user-images.githubusercontent.com/11800206/330554228-cd72b2bd-146a-48e3-9cf2-3312d5ed156b.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjI1OTEzODMsIm5iZiI6MTcyMjU5MTA4MywicGF0aCI6Ii8xMTgwMDIwNi8zMzA1NTQyMjgtY2Q3MmIyYmQtMTQ2YS00OGUzLTljZjItMzMxMmQ1ZWQxNTZiLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA4MDIlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwODAyVDA5MzEyM1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTNiMDAyMDI0NzYwZDg4OWQyYWVhNWFmNGE0ZDYxYjBiMzlhOWY2ZjBiYWE1MzE0ZmIwNGQyZWI1MTkwYzg0M2YmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.Ho-wdg2VbkdYDZK0KDA_kqnmY_4PGYO2gBaxaZf16Bw)
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Pull Request Test Coverage Report for Build 9164902267Details
💛 - Coveralls |
…er into feat/collaborator-tab
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.
Looks great! I've left some comments to check
src/components/WorldListPage/WorldContributorTab/WorldContributorTab.tsx
Show resolved
Hide resolved
|
||
if (!items.length) { | ||
return ( | ||
<Empty className="empty-names-container" height={500}> |
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.
This component has classes, but it doesn't have a module.css
associated with it, is it getting styled by a component which is up the chain?
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.
yes its getting it from the parent component
expect(screen.getByRole('button', { name: t('worlds_list_page.table.unpublish_scene') })).toBeInTheDocument() | ||
}) | ||
|
||
describe('when unpublish button is clicked', () => { |
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.
It seems that the article is missing, would you mind adding it?
describe('when unpublish button is clicked', () => { | |
describe('when the unpublish button is clicked', () => { |
The same goes for other describes and its in the file.
export default function WorldUrl({ ens, deploymentsByWorlds }: Props) { | ||
const url = getExplorerUrl(ens.subdomain) | ||
return isWorldDeployed(deploymentsByWorlds, ens) ? ( | ||
<div className="world-url"> |
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.
What do you think about styling this component with its own WorldUrl.module.css
file?
return render(<WorldUrl ens={ens} deploymentsByWorlds={{}} {...props} />) | ||
} | ||
|
||
describe('when world has a scene deployed', () => { |
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.
It seems that the article is missing, would you mind adding it?
describe('when world has a scene deployed', () => { | |
describe('when a world has a scene deployed', () => { |
The same goes for other its and describes in the file.
src/modules/ens/reducer.spec.ts
Outdated
|
||
describe('when handling fetch contributable names actions', () => { | ||
describe('when handling fetch contributable names request action', () => { | ||
it('should reset contributable names error', () => { |
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.
It seems that the article is missing here, would you mind adding it?
it('should reset contributable names error', () => { | |
it('should reset the contributable names error', () => { |
The same goes for other its and describes in the file.
src/modules/ens/reducer.spec.ts
Outdated
) | ||
}) | ||
|
||
it('should add contributable names to the state', () => { |
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.
it('should add contributable names to the state', () => { | |
it('should remove the loading action from the state', () => { |
src/modules/ens/selectors.spec.ts
Outdated
}) | ||
}) | ||
|
||
describe('when using getContributableNamesError selector', () => { |
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.
It seems that there are some articles missing, would you mind adding them?
describe('when using getContributableNamesError selector', () => { | |
describe('when using the getContributableNamesError selector', () => { |
The same goes for other its and describes in the file.
}) | ||
|
||
const [ownerByNameDomain, ownerByEnsDomain]: [Record<string, string>, Record<string, string>] = yield all([ | ||
call([marketplace, 'fetchENSOwnerByDomain'], nameDomains), |
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.
What do you think about exercising the failure of any of these requests in a test?
…er into feat/collaborator-tab
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.
🚀