-
Notifications
You must be signed in to change notification settings - Fork 59
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
Migrates to Bootstrap 5, closes #266 #267
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #267 +/- ##
==========================================
- Coverage 86.04% 86.00% -0.04%
==========================================
Files 47 47
Lines 3898 3902 +4
==========================================
+ Hits 3354 3356 +2
- Misses 544 546 +2 ☔ View full report in Codecov by Sentry. |
Holy shit, the unit tests! Tomorrow I'll take a look at them, it will be something silly. 😜 Update: Fixed! (at least I think so) |
From my point of view I think I have corrected everything and this PR is ready to merge. If there are any problems, let me know. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you the author of that file? If not, it would be good to make sure to mention its license somewhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is one of the Bootstrap icons (like the rest of the icons on the page).
If I have not forgotten anything with this change Bootstrap is updated to the latest version (right now the 5.3.3). As with the previous version both CSS and JS is embedded in the static directory so you don't have to go looking for these files anywhere.
Before merging this PR it would be best to do some tests in case something goes wrong. I have tested with Firefox and Chrome and everything is fine, but four eyes see more than two.
I have found a bug that is not due to this change (it happens with the current version) and it is that it is not possible to change the email address. From what I have seen it seems to me that the function that handles the post to
changemail
is not complete, it picks up the address that is passed by the form but does nothing with it.Of course, any comments are welcome.