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

Fix issue where carousel controls were no longer showing up #409

Merged
merged 1 commit into from
Feb 12, 2024

Conversation

sea-kelp
Copy link
Collaborator

Description of Changes

The carousel controls are not showing up.

This was caused by a commit that changed paths to face_paths
https://github.com/lucyparsons/OpenOversight/pull/1070/files#diff-cb0d1be5137549bba7d2d657912ea708497d8a757e0623cb76d9d5557fe484e5

But the line controlling whether we show carousel controls is still using paths.

This commit fixes this issue.

Notes for Deployment

None!

Screenshots (if appropriate)

Tests and linting

  • I have rebased my changes on main

  • just lint passes

  • just test passes

@AetherUnbound
Copy link
Collaborator

The only issue with this is that on darker photos, it becomes difficult to tell that there are carousel arrows at all 😞
image
We'll merge this fix and deploy (as it is working for me locally!) but I think some outline on the arrows themselves might be helpful as a follow up. Thanks for fixing this!

@AetherUnbound AetherUnbound merged commit bf47b32 into main Feb 12, 2024
2 checks passed
@AetherUnbound AetherUnbound deleted the fix/carousel-controls branch February 12, 2024 19:42
@sea-kelp
Copy link
Collaborator Author

@AetherUnbound: The arrows are built into the version of Bootstrap we're using unfortunately. Let's circle back to this after the re-design

michplunkett pushed a commit to lucyparsons/OpenOversight that referenced this pull request Feb 12, 2024
<!-- New Contributor? Welcome!

We recommend you check your privacy settings, so the name and email
associated with
the commits are what you want them to be. See the contribution guide at

https://github.com/lucyparsons/OpenOversight/blob/develop/CONTRIB.md#recommended-privacy-settings
for more infos.

Also make sure you have read and abide by the code of conduct:

https://github.com/lucyparsons/OpenOversight/blob/develop/CODE_OF_CONDUCT.md

If this pull request is not ready for review yet, please submit it as a
draft.

Please write your PR name in the present imperative tense. Examples of
that tense are: "Fix issue in the
dispatcher where…", "Improve our handling of…", etc.
-->

## Description of Changes
Copied over from
OrcaCollective#409

The carousel controls are not showing up.

This was caused by a commit that changed `paths` to `face_paths`

https://github.com/lucyparsons/OpenOversight/pull/1070/files#diff-cb0d1be5137549bba7d2d657912ea708497d8a757e0623cb76d9d5557fe484e5

But the line controlling whether we show carousel controls is still
using `paths`.

https://github.com/OrcaCollective/OpenOversight/blob/7bf658c8d431d2db12154ba5c90017d46f19cf14/OpenOversight/app/templates/partials/officer_faces.html#L21

This commit fixes this issue.

## Notes for Deployment
N/A

## Screenshots (if appropriate)


## Tests and Linting
 - [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.

2 participants