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

[App] Add presences list #186

Merged
merged 11 commits into from
Jul 6, 2023
Merged

[App] Add presences list #186

merged 11 commits into from
Jul 6, 2023

Conversation

Afonso-santos
Copy link
Contributor

I created a page that consists of being a list of attendance of the sessions.

@netlify
Copy link

netlify bot commented Jun 27, 2023

Deploy Preview for coderdojobraga-blog canceled.

Name Link
🔨 Latest commit 0169654
🔍 Latest deploy log https://app.netlify.com/sites/coderdojobraga-blog/deploys/64a6d55307080f00080560f6

@netlify
Copy link

netlify bot commented Jun 27, 2023

Deploy Preview for coderdojobraga-web canceled.

Name Link
🔨 Latest commit 0169654
🔍 Latest deploy log https://app.netlify.com/sites/coderdojobraga-web/deploys/64a6d553a732d60008859a01

@netlify
Copy link

netlify bot commented Jun 27, 2023

Deploy Preview for coderdojobraga-maintenance canceled.

Name Link
🔨 Latest commit 0169654
🔍 Latest deploy log https://app.netlify.com/sites/coderdojobraga-maintenance/deploys/64a6d5532031d60007b7623b

@netlify
Copy link

netlify bot commented Jun 27, 2023

Deploy Preview for coderdojobraga-app ready!

Name Link
🔨 Latest commit 0169654
🔍 Latest deploy log https://app.netlify.com/sites/coderdojobraga-app/deploys/64a6d5534a33ed0008b0b8c0
😎 Deploy Preview https://deploy-preview-186--coderdojobraga-app.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@ruioliveira02 ruioliveira02 left a comment

Choose a reason for hiding this comment

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

Format your code with npm run lint if you haven't done so already.

Don't worry about the link checker action failing, we are fixing it in another PR

Comment on lines 18 to 42
type Event = {
id: string;
title: string;
location_id: string;
};

type Lecture = {
id: string;
event: Event;
mentor: {
id: string;
first_name: string;
last_name: string;
photo: string;
};
ninja: {
id: string;
first_name: string;
last_name: string;
photo: string;
};
summary: string;
notes: string;
attendance: string;
};
Copy link
Member

Choose a reason for hiding this comment

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

This type definitions are more generic than this page. I think we can move them to the bokkenjs package.

useEffect(() => {
generateData();
}, [selectedLectures]);
console.log(selectedLectures);
Copy link
Member

Choose a reason for hiding this comment

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

Remove this print :)

Copy link
Member

@ruioliveira02 ruioliveira02 left a comment

Choose a reason for hiding this comment

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

When I finish editing a given session, and then move to a different one and back to the one I just edited, the changes are not being reflected. I believe it is because you are not updating your local copy of the lecture after you save it.

Also rebase with main

git rebase origin/main
git push -f # Only force push when strictly necessary :) 

That should fix the Link Checker action

api
.listEvents()
.then((response: any) => setEvents(response.data))
.catch(() => {});
Copy link
Member

Choose a reason for hiding this comment

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

Add a notification for this error. The worst thing you can do is having the request fail silently.

api
.getLocations()
.then((response: any) => setLocations(response.data))
.catch(() => {});
Copy link
Member

Choose a reason for hiding this comment

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

Same thing here

Copy link
Member

@ruioliveira02 ruioliveira02 left a comment

Choose a reason for hiding this comment

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

Functionally looks very good to me

Comment on lines 208 to 223
<Upload
name="avatar"
accept="image/*"
beforeUpload={(file) => {
getBase64(file, (imageUrl: any) => setAvatar(imageUrl));
return false;
}}
onRemove={() => setAvatar(null)}
multiple={false}
maxCount={1}
showUploadList={{
showDownloadIcon: false,
showPreviewIcon: false,
showRemoveIcon: true,
}}
>
<Button icon={<UploadOutlined />}>Selecionar</Button>
</Upload>
<ImgCrop>
<Upload
name="avatar"
accept="image/*"
beforeUpload={(file) => {
getBase64(file, (imageUrl: any) => setAvatar(imageUrl));
return false;
}}
onRemove={() => setAvatar(null)}
multiple={false}
maxCount={1}
showUploadList={{
showDownloadIcon: false,
showPreviewIcon: false,
showRemoveIcon: true,
}}
Copy link
Member

Choose a reason for hiding this comment

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

Aren't these changes duplicated from #181 ? If so it should be removed as it will be automatically updated when that PR gets merged

Comment on lines 42 to 55
// const onFinish = (values: any, lectureId: string) => {
// api

// .updateLecture(lectureId, values)
// .then(() => {
// notifyInfo("Os dados da sessão foram atualizados com sucesso", "");
// })
// .catch((error) => {
// notifyError(
// "Ocorreu um erro",
// "Não foi possível atualizar os dados da sessão"
// );
// });
// };
Copy link
Member

Choose a reason for hiding this comment

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

Remove this if you no longer need it

Copy link
Member

@ruioliveira02 ruioliveira02 left a comment

Choose a reason for hiding this comment

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

Nice work 🚀

@Afonso-santos Afonso-santos merged commit 0173124 into main Jul 6, 2023
@Afonso-santos Afonso-santos deleted the as/presences_list branch July 6, 2023 15:38
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.

2 participants