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

Upgrade to Bootstrap 5 #458

Merged
merged 5 commits into from
Jun 22, 2024
Merged

Upgrade to Bootstrap 5 #458

merged 5 commits into from
Jun 22, 2024

Conversation

sea-kelp
Copy link
Collaborator

Description of Changes

Upgrade to Bootstrap 5 in preparation for upcoming changes. Everything looks slightly different due to changes in the bootstrap framework.

Notes for Deployment

None

Screenshots (if appropriate)

1

Tests and linting

Manually checked that all pages look roughly reasonably similar to the previous look

  • I have rebased my changes on main

  • just lint passes

  • just test passes

@sea-kelp sea-kelp requested a review from a team as a code owner June 17, 2024 06:37
@@ -82,7 +82,7 @@ repos:
- --quiet

- repo: https://github.com/PyCQA/flake8
rev: 6.0.0
rev: 7.0.0
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Needed since the old version was incorrectly flagging correct f-string syntax

@@ -60,7 +60,7 @@ def before_request():
and not current_user.confirmed
and request.endpoint
and request.endpoint[:5] != "auth."
and request.endpoint != "static"
and request.endpoint not in ["static", "bootstrap.static"]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Exclude the bootstrap.static folder (used by the new bootstrap-flask dependency to serve assets) so stylesheets and scripts load correctly when the user is unconfirmed

@@ -23,6 +23,7 @@
class BaseConfig:
def __init__(self):
# App Settings
self.BOOTSTRAP_SERVE_LOCAL = True
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Serve Bootstrap assets locally instead of from a CDN

Comment on lines +25 to +26
<div id="step-1"
class="card mb-5"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Replacing the step-by-step dialogue in favor with a fully laid-out form since the old version was broken by the upgrade and this reduces user clicks and increases form completion

Copy link

@catgirlcoded catgirlcoded left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Collaborator

@AetherUnbound AetherUnbound left a comment

Choose a reason for hiding this comment

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

Incredible!! This is a huge effort, so exciting to see us move off of bootstrap 3 🤩

@AetherUnbound AetherUnbound merged commit 9f2b5c4 into main Jun 22, 2024
2 checks passed
@AetherUnbound AetherUnbound deleted the feature/bootstrap-5 branch June 22, 2024 21:21
michplunkett pushed a commit to lucyparsons/OpenOversight that referenced this pull request Jun 25, 2024
## Description of Changes
*This change is cherry-picked from
OrcaCollective#458
Upgrade to Bootstrap 5 in preparation for upcoming changes. Everything
looks slightly different due to changes in the bootstrap framework.

## Notes for Deployment
None

## Screenshots (if appropriate)

![1](https://github.com/lucyparsons/OpenOversight/assets/66500457/f4c167a9-b7f4-43bb-a5d3-c2a3ca3a75fe)


## Tests and Linting
Manually checked that all pages look roughly reasonably similar to the
previous look

 - [x] This branch is up-to-date with the `develop` branch.
 - [x] `pytest` passes on my local development environment.
 - [x] `pre-commit` passes on my local development environment.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants