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: add creatives table and sectors page #23

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sebas-memorable
Copy link

As requested in the FE test, I added the creatives table in Library page and added a new sectors page.

I created the corresponding .domain, .repository, .page and . widget files and added them trying to follow current project structure. I tried to reuse as much components as I could, in order to not duplicated things.

I had to update schemas.graphql and schemas.ts since the listFolder BE service was replaced by folder. So I had to adjust the query name and add the new types.

I added a date formatter tool to format dates. This tool uses the locale to format the date. By default it uses en-US locale but by passing user's locale as the second parameter, you can get the date formatted in user's locale. I also added some unit test for this.

I had to remove omick dependency from package.json since it wasn't available in yarn repository. Nonetheless, that dependency was not being used.

This is the first time I see this kind of project structure for a React project. It took me a while to understand where each component lives. Even though I see the value provided by this folder structure, I think for a React project there's a more simple approach.

Another thing I found interesting is the use of genql together with swr to communicate with the GQL server. Having to create several files in order to call a GQL query (for instance). I think the same thing could be addressed with apollo client with lot less code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant