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

Add rotate icon and crop button in Lightbox #48

Merged
merged 9 commits into from
Jul 2, 2024

Conversation

jonathan-waarneming-nl
Copy link
Collaborator

@jonathan-waarneming-nl jonathan-waarneming-nl commented Jun 26, 2024

@jonathan-waarneming-nl jonathan-waarneming-nl changed the title Feature/crop icons Add rotate icon and crop button in Lightbox Jun 26, 2024
@jonathan-waarneming-nl jonathan-waarneming-nl marked this pull request as ready for review June 26, 2024 17:42
@jonathan-waarneming-nl
Copy link
Collaborator Author

@SjaakSchilperoort Zou je hier naar willen kijken?

src/lib/Icons.ts Outdated
@@ -241,6 +243,7 @@ type IconName = Extract<
| 'images'
| 'info-circle'
| 'language'
| 'level-down'
Copy link
Member

Choose a reason for hiding this comment

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

De naam in Font Awesome is arrow-turn-down. Waarom gebruik je die naam niet?

Ik zie dat Karel in het design arrow-turn-down 90 graden naar links draait, maar je kunt dan toch net zo goed arrow-turn-right gebruiken? Dan hebben we (nog) geen support voor rotatie nodig.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Idd. Ik wist niet dat die ook bestond. Zal dat doen.

@@ -42,6 +42,7 @@ const getLightboxFooterComponent =
content?: React.ReactNode,
style?: LightboxStyle,
onPressDelete?: () => void,
onPressCrop?: () => void,
Copy link
Member

Choose a reason for hiding this comment

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

Unit tests hiervoor ontbreken nog

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Idd, zal ze toevoegen

Copy link
Member

@SjaakSchilperoort SjaakSchilperoort left a comment

Choose a reason for hiding this comment

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

👍🏻

@jonathan-waarneming-nl jonathan-waarneming-nl merged commit a2415a2 into develop Jul 2, 2024
2 checks passed
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