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

Epic 13: Kai Notification #138

Open
wants to merge 27 commits into
base: develop
Choose a base branch
from

Conversation

DanielDaCosta
Copy link

@DanielDaCosta DanielDaCosta commented Aug 29, 2024

Description

This epic focuses on enhancing the user experience for Kai, an AI teaching assistant, by implementing a series of toast notifications and error pages.

New features implemented:

  1. Implementing Login Notifications (@DanielDaCosta)
    This task focuses on implementing notification features that inform users of their login attempt status. The goal is to design and integrate toast notifications in the user interface, ensuring they are triggered upon successful or failed login attempts via Firebase Auth.
  2. Implementing Signup Notifications (@rajmal-sk, @yhannineh)
    This task focuses on implementing a notification feature that informs users of a successful sign-up attempt. The goal is to design and integrate a toast notification in the user interface, ensuring that it is triggered upon successful registration via Firebase Auth.
  3. Implementing Error Pages (@adhamkhallaf06, @kimberlytr)
    This task focuses on creating user-friendly error pages that inform users when various errors occur. The goal is to design and integrate error pages in the user interface, providing clear messages and actionable steps for the user.
  4. Implementing Quiz Creation Error Notifications (@r-mallick, @txsjyy)
    This task focuses on implementing notification features that inform users of errors during quiz creation. The goal is to design and integrate toast notifications in the user interface, ensuring they are triggered upon detecting specific errors like missing PDF files.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • [x ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

DanielDaCosta and others added 27 commits July 13, 2024 21:27
Uses the Youtube Data API v3 to check if videos that are uploaded are too long for the application to handle.
Task 6 tool error notifications -  Code reviewed and approved by @kimberlytr and @adhamkhallaf06
@bkb-Git bkb-Git changed the base branch from main to develop September 2, 2024 13:55
Copy link
Contributor

@bkb-Git bkb-Git left a comment

Choose a reason for hiding this comment

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

Dropped a couple of comments that may need to be resolved and please attach a loom video showcasing the different features built in the PR.

@@ -1,12 +1,12 @@
{
"projects": {
"default": "kai-ai-f63c8"
"default": "kai-platform-20960"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please undo this

},
"targets": {
"kai-ai-f63c8": {
"kai-platform-20960": {
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above, undo

Copy link
Contributor

Choose a reason for hiding this comment

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

Restore this file

error?.message || 'Couldn\u0027t send prompt'
);
// Helper function to parse ISO 8601 duration to seconds
const parseDuration = (isoDuration) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Make use of the moment library which makes it easier to work with dates and time

return hours * 3600 + minutes * 60 + seconds;
};

const handleSubmitForm = async (values) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically this check is done on the AI side, would like to know why this logic has been introduced

} = props;

const closeButton = (
<React.Fragment>
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no need for fragments here given that there is only one child element

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be better to simply export a single component from this file that can be used dynamically instead of exporting these two components which are mostly similar.

return () => {
// Cleanup actions if necessary
};
}, []);
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the purpose of the above useEffect, the handleError function is not being called anywhere within the useEffect

};

if (hasError) {
if (error && error.message && error.message.includes('Network Error')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Move strings to a constant file and reference it from three

if (error && error.message && error.message.includes('Network Error')) {
return <NetworkErrorPage retry={handleRetry} />;
}
if (error && error.message && error.message.includes('404')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above comment

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.

6 participants