Skip to content

Conversation

@Marrufof
Copy link

Changes in this PR

  • Added 2 editable content areas

    • loggedOutFooter
    • loggedInFooter (note: the version number is not editable)
  • Header Logo
    To change the header logo, add images to: RIT-SE-Senior-Project/ui/src/Assets

  • Footer Logo
    To change the footer logo, add images to: RIT-SE-Senior-Project\ui\public\assets

@Rtyujklop
Copy link
Contributor

Hey I went through everything it looks good

Copy link

Choose a reason for hiding this comment

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

This information seems like it may be irrelevant to the README file in the root directory. Consider moving the information about the content editor to a .md markdown file in the test_cases directory.

Copy link
Author

Choose a reason for hiding this comment

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

I will double check, I believe the professor wanted it like this.

Copy link

@atauln atauln Dec 5, 2025

Choose a reason for hiding this comment

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

I would recommend removing this from the pull request, as there are no changes on it.

})
.catch((error) => {
console.error("Error fetching footer:", error);
setFooterHtml("");
Copy link

Choose a reason for hiding this comment

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

Instead of completely removing the footer's HTML, maybe we could leave an error message?

Copy link
Author

Choose a reason for hiding this comment

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

Yes I can look into that! We could discuss this Friday too!

Copy link

Choose a reason for hiding this comment

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

I appreciate the hardcoded strings and configuration variables being centered in this file! This should be great for maintainability moving forward.

not used/ empty
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Needs Planning

Development

Successfully merging this pull request may close these issues.

6 participants