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: replace print with borders function with panel and change color to red in output.py #136

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

suppo01
Copy link
Collaborator

@suppo01 suppo01 commented Oct 19, 2024

Adding Panel for UI Use

Description

I added the implementation of panel to the output.py file to replace the old system, the print with border function. With this, I also deleted the old function as it was no longer being used by any function in output.py or beyond. I also made the color of the panel displaying the checks to be red instead of white if the checks are not 100% complete. This makes it easier to distinguish the information from all the surrounding white text.

Type of Change

  • [X ] Feature
  • Bug fix
  • Documentation

Contributors

Reminder

  • All GitHub Actions should be in a passing state before any pull request is merged.
  • All PRs must be reviewed by at least one team member and one member of the Integration team!
  • Any issues this PR closes are tagged in the description!

@gkapfham
Copy link
Collaborator

Hello @suppo01 can you please furnish screenshots that show what this looks like at:

  • All relevant sizes for the box (i.e., small and large sized terminal windows)
  • All operating systems (i.e., MacOS, Windows, and Linux)
  • At least or or two different terminal windows

I think that the reviewers of this PR are going to need to see additional context about this feature before they agree to merge it because of the fact that it is visual in nature.

@gkapfham
Copy link
Collaborator

Hello @suppo01 please note that the build is failing for this PR. Can you please investigate this issue and try to resolve it? None of the members of our team will agree to review and then ultimately approve this PR for merging until the build issues are resolved.

@gkapfham
Copy link
Collaborator

@suppo01 please identify two student members of our team for review and one student technical leader for review and tag them as the reviewers for this PR.

@gkapfham gkapfham added the enhancement New feature or request label Oct 24, 2024
@rebekahrudd rebekahrudd self-requested a review October 24, 2024 19:26
@rebekahrudd
Copy link
Collaborator

I just ran and tested this code on my Linux computer running Ubuntu 22.04.4 LTS.

Steps:

  1. I entered Molly's fork
  2. I installed the forked and added a print statement to make sure it was Molly's repo
  3. Then I pipx installed her version of gatorgrade

I tested gatorgrade in both a large terminal window and a small terminal window

Large terminal window:
large_terminal

Small terminal window:
small_terminal

Good news! This update for the box that displays the results works in both a large and small terminal window for my Linux window!

@suppo01
Copy link
Collaborator Author

suppo01 commented Oct 24, 2024

Large Terminal Window on Windows:

Screenshot 2024-10-24 153750

Small Terminal Window on Windows:

Screenshot 2024-10-24 153904

Copy link
Collaborator

@rebekahrudd rebekahrudd left a comment

Choose a reason for hiding this comment

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

This code works on my Linux computer and displays correctly. Code looks well documented and understandable for future users.

@rebekahrudd rebekahrudd requested a review from gkapfham October 24, 2024 19:43
Copy link
Collaborator

@CalebKendra CalebKendra left a comment

Choose a reason for hiding this comment

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

Looks good!

@Chezka109 Chezka109 self-requested a review October 28, 2024 14:10
Copy link
Collaborator

@Chezka109 Chezka109 left a comment

Choose a reason for hiding this comment

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

LGTM! I tried it on MacOS and works perfectly. Screenshots are below:

Screenshot 2024-10-28 at 10 07 18 AM Screenshot 2024-10-28 at 10 07 44 AM

@AlishChhetri AlishChhetri self-requested a review November 7, 2024 20:13
Copy link
Collaborator

@AlishChhetri AlishChhetri left a comment

Choose a reason for hiding this comment

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

LGTM!

@gkapfham
Copy link
Collaborator

Thanks @suppo01 for this PR, I will review it a final time and likely merge it this week. Please note that I need to first address some PRs for another project before I can return to merging PRs for this project. Let me know if you have questions, okay?

@suppo01
Copy link
Collaborator Author

suppo01 commented Dec 5, 2024

The version is now 0.7.1 as agreed in class to be released using Pallas's automatic release feature.

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

Successfully merging this pull request may close these issues.

6 participants