Add landscape composite horizon directions and annotation toggle #22
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| { | ||
| "name": "@eclipse-timer/mobile", | ||
| "version": "1.1.38", | ||
| "private": true, | ||
| "main": "index.js", | ||
| "scripts": { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -34,6 +34,30 @@ const TOTALITY_MOON_COLOR = TOTALITY_SKY_COLOR; | |||||
| const TOTALITY_MOON_BORDER_COLOR = "#1f2c47"; | ||||||
| const TOTALITY_HORIZON_LINE_COLOR = "rgba(255, 174, 205, 0.75)"; | ||||||
| const TOTALITY_HORIZON_GLOW_COLOR = "rgba(255, 136, 182, 0.42)"; | ||||||
| const LANDSCAPE_HORIZONTAL_FOV_DEG_24MM = 74; | ||||||
| const COMPASS_MARKERS = [ | ||||||
| { label: "N", azimuthDeg: 0 }, | ||||||
| { label: "NNE", azimuthDeg: 22.5 }, | ||||||
| { label: "NE", azimuthDeg: 45 }, | ||||||
| { label: "ENE", azimuthDeg: 67.5 }, | ||||||
| { label: "E", azimuthDeg: 90 }, | ||||||
| { label: "ESE", azimuthDeg: 112.5 }, | ||||||
| { label: "SE", azimuthDeg: 135 }, | ||||||
| { label: "SSE", azimuthDeg: 157.5 }, | ||||||
| { label: "S", azimuthDeg: 180 }, | ||||||
| { label: "SSW", azimuthDeg: 202.5 }, | ||||||
| { label: "SW", azimuthDeg: 225 }, | ||||||
| { label: "WSW", azimuthDeg: 247.5 }, | ||||||
| { label: "W", azimuthDeg: 270 }, | ||||||
| { label: "WNW", azimuthDeg: 292.5 }, | ||||||
| { label: "NW", azimuthDeg: 315 }, | ||||||
| { label: "NNW", azimuthDeg: 337.5 }, | ||||||
| ] as const; | ||||||
|
|
||||||
| function normalizeSignedDeltaDeg(fromDeg: number, toDeg: number) { | ||||||
| const delta = ((toDeg - fromDeg + 540) % 360) - 180; | ||||||
| return delta === -180 ? 180 : delta; | ||||||
| } | ||||||
|
Comment on lines
+57
to
+60
There was a problem hiding this comment.
There was a problem hiding this comment. You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
||||||
|
|
||||||
| export type PhotographyGuidePayload = { | ||||||
| eclipseId: string; | ||||||
|
|
@@ -89,6 +113,7 @@ export default function PhotographyGuideScreen({ | |||||
| const [totalPictures, setTotalPictures] = useState<PhotographyGuidePictureCount>(5); | ||||||
| const [isCountPickerOpen, setIsCountPickerOpen] = useState(false); | ||||||
| const [isLandscapeCompositeOpen, setIsLandscapeCompositeOpen] = useState(false); | ||||||
| const [showCompositeMarkings, setShowCompositeMarkings] = useState(true); | ||||||
| const [compositeStageSize, setCompositeStageSize] = useState({ | ||||||
| width: 0, | ||||||
| height: 0, | ||||||
|
|
@@ -184,6 +209,22 @@ export default function PhotographyGuideScreen({ | |||||
| if (typeof activeCompositeHorizonY !== "number") return undefined; | ||||||
| return Math.max(0, compositeStageSize.height - activeCompositeHorizonY); | ||||||
| }, [activeCompositeHorizonY, compositeStageSize.height]); | ||||||
| const horizonCompassMarkers = useMemo(() => { | ||||||
| if (!compositeLayout) return []; | ||||||
| const maxPlacement = compositeLayout.placements.find( | ||||||
| (placement) => placement.phaseBucket === "MAX" && typeof placement.sunAzimuthDeg === "number", | ||||||
| ); | ||||||
| if (!maxPlacement || typeof maxPlacement.sunAzimuthDeg !== "number") return []; | ||||||
| const centerAzimuthDeg = maxPlacement.sunAzimuthDeg; | ||||||
|
|
||||||
| return COMPASS_MARKERS.map((marker) => { | ||||||
| const deltaDeg = normalizeSignedDeltaDeg(centerAzimuthDeg, marker.azimuthDeg); | ||||||
| const x = | ||||||
| compositeLayout.anchorX + | ||||||
| (deltaDeg / LANDSCAPE_HORIZONTAL_FOV_DEG_24MM) * compositeStageSize.width; | ||||||
| return { ...marker, x, inFrame: x >= 0 && x <= compositeStageSize.width }; | ||||||
| }).filter((marker) => marker.inFrame); | ||||||
| }, [compositeLayout, compositeStageSize.width]); | ||||||
| const handleCompositeStageLayout = (event: LayoutChangeEvent) => { | ||||||
| const nextWidth = Math.round(event.nativeEvent.layout.width); | ||||||
| const nextHeight = Math.round(event.nativeEvent.layout.height); | ||||||
|
|
@@ -363,6 +404,16 @@ export default function PhotographyGuideScreen({ | |||||
| /> | ||||||
| <View style={styles.compositeModal}> | ||||||
| <Text style={styles.compositeModalTitle}>Landscape composite</Text> | ||||||
| <Pressable | ||||||
| style={styles.compositeMarkingsToggleBtn} | ||||||
| onPress={() => setShowCompositeMarkings((current) => !current)} | ||||||
| accessibilityRole="button" | ||||||
| accessibilityLabel="Toggle composite markings" | ||||||
| > | ||||||
| <Text style={styles.compositeMarkingsToggleText}> | ||||||
| {showCompositeMarkings ? "Hide" : "Show"} markings, directions, and shot numbers | ||||||
|
c0d3rb4b4 marked this conversation as resolved.
|
||||||
| </Text> | ||||||
| </Pressable> | ||||||
| <View style={styles.compositeFrame} onLayout={handleCompositeStageLayout}> | ||||||
| <View | ||||||
| style={[ | ||||||
|
|
@@ -402,6 +453,26 @@ export default function PhotographyGuideScreen({ | |||||
| : null, | ||||||
| ]} | ||||||
| /> | ||||||
| {showCompositeMarkings && typeof activeCompositeHorizonY === "number" | ||||||
| ? horizonCompassMarkers.map((marker) => ( | ||||||
| <View key={`horizon-marker-${marker.label}`} style={{ left: marker.x - 10 }}> | ||||||
| <View | ||||||
| style={[ | ||||||
| styles.compositeDirectionTick, | ||||||
| { top: activeCompositeHorizonY - 6 }, | ||||||
|
There was a problem hiding this comment. The horizon compass marker wrapper is offset by
Suggested change
There was a problem hiding this comment. Summary
Testing
|
||||||
| ]} | ||||||
| /> | ||||||
| <Text | ||||||
| style={[ | ||||||
| styles.compositeDirectionLabel, | ||||||
| { top: activeCompositeHorizonY + 4 }, | ||||||
| ]} | ||||||
| > | ||||||
| {marker.label} | ||||||
| </Text> | ||||||
| </View> | ||||||
| )) | ||||||
| : null} | ||||||
| {compositeLayout ? ( | ||||||
| <> | ||||||
| <View | ||||||
|
|
@@ -415,92 +486,98 @@ export default function PhotographyGuideScreen({ | |||||
| /> | ||||||
| {compositeLayout.placements.map((placement) => ( | ||||||
| <View key={placement.index}> | ||||||
| {!placement.isAboveHorizon ? null : ( | ||||||
| <> | ||||||
| {isTotalCompositeTheme && | ||||||
| placement.phaseBucket === "MAX" && | ||||||
|
c0d3rb4b4 marked this conversation as resolved.
|
||||||
| placement.showMoon && | ||||||
| placement.moon ? ( | ||||||
| <> | ||||||
| <View | ||||||
| style={[ | ||||||
| styles.compositeCoronaGlow, | ||||||
| { | ||||||
| width: Math.max(placement.sunRadius * 9, 16), | ||||||
| height: Math.max(placement.sunRadius * 9, 16), | ||||||
| borderRadius: Math.max(placement.sunRadius * 4.5, 8), | ||||||
| left: placement.x - Math.max(placement.sunRadius * 4.5, 8), | ||||||
| top: placement.y - Math.max(placement.sunRadius * 4.5, 8), | ||||||
| }, | ||||||
| ]} | ||||||
| /> | ||||||
| <View | ||||||
| style={[ | ||||||
| styles.compositeCoronaRing, | ||||||
| { | ||||||
| width: Math.max(placement.sunRadius * 6, 10), | ||||||
| height: Math.max(placement.sunRadius * 6, 10), | ||||||
| borderRadius: Math.max(placement.sunRadius * 3, 5), | ||||||
| left: placement.x - Math.max(placement.sunRadius * 3, 5), | ||||||
| top: placement.y - Math.max(placement.sunRadius * 3, 5), | ||||||
| }, | ||||||
| ]} | ||||||
| /> | ||||||
| </> | ||||||
| ) : null} | ||||||
| <View | ||||||
| style={[ | ||||||
| styles.compositeSun, | ||||||
| { | ||||||
| width: placement.sunRadius * 2, | ||||||
| height: placement.sunRadius * 2, | ||||||
| borderRadius: placement.sunRadius, | ||||||
| left: placement.x - placement.sunRadius, | ||||||
| top: placement.y - placement.sunRadius, | ||||||
| }, | ||||||
| ]} | ||||||
| /> | ||||||
| {placement.showMoon && placement.moon ? ( | ||||||
| <View | ||||||
| style={[ | ||||||
| styles.compositeMoon, | ||||||
| isTotalCompositeTheme | ||||||
| ? { | ||||||
| backgroundColor: TOTALITY_MOON_COLOR, | ||||||
| borderColor: TOTALITY_MOON_BORDER_COLOR, | ||||||
| } | ||||||
| : null, | ||||||
| { | ||||||
| width: placement.moon.radius * 2, | ||||||
| height: placement.moon.radius * 2, | ||||||
| borderRadius: placement.moon.radius, | ||||||
| left: placement.moon.x - placement.moon.radius, | ||||||
| top: placement.moon.y - placement.moon.radius, | ||||||
| }, | ||||||
| ]} | ||||||
| /> | ||||||
| ) : null} | ||||||
| {showCompositeMarkings ? ( | ||||||
| <View | ||||||
| style={[ | ||||||
| styles.compositeShotIndexTag, | ||||||
| { | ||||||
| left: placement.x - 9, | ||||||
| top: placement.y + placement.sunRadius + 3, | ||||||
| }, | ||||||
| placement.clamped ? styles.compositeShotIndexTagClamped : null, | ||||||
| ]} | ||||||
| > | ||||||
| <Text style={styles.compositeShotIndexText}>{placement.index}</Text> | ||||||
| </View> | ||||||
| ) : null} | ||||||
| {showCompositeMarkings && placement.clamped ? ( | ||||||
| <View | ||||||
| style={[ | ||||||
| styles.compositeClampIndicator, | ||||||
| { | ||||||
| left: placement.x + placement.sunRadius - 4, | ||||||
| top: placement.y - placement.sunRadius - 4, | ||||||
| }, | ||||||
| ]} | ||||||
| /> | ||||||
| ) : null} | ||||||
| </> | ||||||
| )} | ||||||
| </View> | ||||||
| ))} | ||||||
| </> | ||||||
|
|
@@ -510,7 +587,8 @@ export default function PhotographyGuideScreen({ | |||||
| 24mm framing simulation with MAX anchored at frame center. | ||||||
| </Text> | ||||||
| <Text style={styles.compositeLegendText}> | ||||||
| Numbers are shot indices. Horizon ticks show compass directions. Amber dots mark | ||||||
| edge-clamped shots. | ||||||
| </Text> | ||||||
| <Pressable | ||||||
| style={styles.compositeModalCloseBtn} | ||||||
|
|
@@ -903,6 +981,36 @@ function createStyles(colors: ReturnType<typeof useAppTheme>["colors"]) { | |||||
| borderColor: "#7a5222", | ||||||
| opacity: 0.9, | ||||||
| }, | ||||||
| compositeDirectionTick: { | ||||||
| position: "absolute", | ||||||
| width: 1, | ||||||
| height: 12, | ||||||
| backgroundColor: "rgba(255,255,255,0.7)", | ||||||
| }, | ||||||
| compositeDirectionLabel: { | ||||||
| position: "absolute", | ||||||
| width: 20, | ||||||
| textAlign: "center", | ||||||
| color: "#ffffff", | ||||||
| fontSize: 9, | ||||||
| fontWeight: "700", | ||||||
| textShadowColor: "rgba(0,0,0,0.4)", | ||||||
| textShadowRadius: 2, | ||||||
| textShadowOffset: { width: 0, height: 1 }, | ||||||
| }, | ||||||
| compositeMarkingsToggleBtn: { | ||||||
| borderRadius: 10, | ||||||
| borderWidth: 1, | ||||||
| borderColor: colors.inputBorder, | ||||||
| backgroundColor: colors.surfaceMuted, | ||||||
| paddingVertical: 8, | ||||||
| paddingHorizontal: 10, | ||||||
| }, | ||||||
| compositeMarkingsToggleText: { | ||||||
| color: colors.textSecondary, | ||||||
| fontSize: 12, | ||||||
| fontWeight: "700", | ||||||
| }, | ||||||
| compositeModal: { | ||||||
| width: "100%", | ||||||
| borderRadius: 12, | ||||||
|
|
||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
LANDSCAPE_HORIZONTAL_FOV_DEG_24MMis defined here and also inapps/mobile/src/utils/photographyGuide.ts(same value). Duplicating this constant risks the UI compass markers drifting out of sync with the layout math if one side changes. Consider exporting the FOV constant from the utils module (or a shared constants module) and reusing it here.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.
@codex Consider this
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.
Summary
photographyGuide.ts(LANDSCAPE_HORIZONTAL_FOV_DEG_24MMandnormalizeSignedDeltaDeg). apps/mobile/src/screens/PhotographyGuideScreen.tsxL17-L24 apps/mobile/src/utils/photographyGuide.tsL51-L53 apps/mobile/src/utils/photographyGuide.tsL119-L122left: 10inside the wrapper). apps/mobile/src/screens/PhotographyGuideScreen.tsxL453-L471showCompositeMarkings. apps/mobile/src/screens/PhotographyGuideScreen.tsxL484-L578aa270c4) and created a follow-up PR viamake_pr.Testing
pnpm -C apps/mobile test tests/photography-guide.test.tspnpm -C apps/mobile typecheckN/A (React Native screen change; browser screenshot capture is not applicable in this environment)View task →