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

Update Gradebook user interface #16

Merged
merged 8 commits into from
Nov 28, 2023
Merged

Conversation

turtlegarden
Copy link
Contributor

Preview

Skarminspelning.fran.2023-10-30.00-09-51.webm

Explanation

This pull request completely remakes the UI of Gradebook, updating it to the latest GNOME 45 HIG.

Downfalls

  • For some reason, adding a category after a subject is created does not work. I am unsure why this happens. Everything else should work very well!
  • I removed the Date function on Grades because it would take too much effort to create a date widget for something that does not need dating (and if this is really needed, one can put it in the note).

I hope you enjoy!

Note: for development purposes, there is a flatpak build JSON compatible with GNOME Builder included. Please tell me if it works when built on the yaml and if I should remove the .json file!

@turtlegarden
Copy link
Contributor Author

Hello, any updates on this?

@turtlegarden
Copy link
Contributor Author

Hello, it has been three weeks yet there is no response to this patch. Could you please respond with a status update?

@turtlegarden
Copy link
Contributor Author

This pull request is stale, I am closing it. Feel free to review it if this app is still maintained. I would have appreciated some sort of reply.

Copy link
Owner

@leolost2605 leolost2605 left a comment

Choose a reason for hiding this comment

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

First of all thanks a lot for this massive PR! Second I am very sorry I didn't reply sooner I'm currently quite occupied with university and admittedly kind of forgot about this :(

If you are still interested in working on this here are some things I saw while quickly looking over it:

  • The .flatpak-builder folder shouldn't be included in git
  • While running I get some criticals which isn't very good:
(io.github.leolost2605.gradebook:2): Gtk-CRITICAL **: 18:52:56.380: gtk_widget_get_name: assertion 'GTK_IS_WIDGET (widget)' failed

(io.github.leolost2605.gradebook:2): Gtk-CRITICAL **: 18:52:56.380: gtk_widget_get_name: assertion 'GTK_IS_WIDGET (widget)' failed

(io.github.leolost2605.gradebook:2): Adwaita-CRITICAL **: 18:53:11.849: adw_window_set_content: assertion 'gtk_widget_get_parent (content) == NULL' failed

(io.github.leolost2605.gradebook:2): Gtk-CRITICAL **: 18:53:11.849: gtk_box_remove: assertion 'GTK_IS_WIDGET (child)' failed

(io.github.leolost2605.gradebook:2): Gtk-CRITICAL **: 18:53:11.849: gtk_box_remove: assertion 'GTK_IS_WIDGET (child)' failed

(io.github.leolost2605.gradebook:2): Gtk-CRITICAL **: 18:53:11.851: gtk_box_append: assertion 'GTK_IS_WIDGET (child)' failed

(io.github.leolost2605.gradebook:2): Adwaita-CRITICAL **: 18:53:17.866: adw_window_set_content: assertion 'gtk_widget_get_parent (content) == NULL' failed

I'll try to do a detailed review ASAP!
Apart from that it looks very good and the new UI looks amazing! Great work! 🎉

src/application.vala Outdated Show resolved Hide resolved
src/application.vala Outdated Show resolved Hide resolved
src/application.vala Show resolved Hide resolved
Copy link
Owner

@leolost2605 leolost2605 left a comment

Choose a reason for hiding this comment

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

Ok nevermind the detailed review I just remembered how unorganized I did things back then and what an ugly codebase this is so I think it's better to just merge this and go to cleaning up the whole thing later 😅

So if it's ok with you I'll remove the .flatpak-builder folder from git and the merge this :)

@turtlegarden
Copy link
Contributor Author

Oh no, the .flatpak-builder folder made it in? That is not good, I will delete it right now, as well as apply your changes :)

@turtlegarden
Copy link
Contributor Author

Would it be okay if I run a formatter on this? My formatting is all over the place 😅️

@leolost2605
Copy link
Owner

Sure go ahead! Although I think we might want to run a formatter on the whole codebase as it's formatting is all over the place anyways 😅

@leolost2605 leolost2605 merged commit f1b4758 into leolost2605:main Nov 28, 2023
1 check failed
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