Skip to content

Conversation

Jon-edge
Copy link
Collaborator

@Jon-edge Jon-edge commented Sep 1, 2025

image image image

CHANGELOG

Does this branch warrant an entry to the CHANGELOG?

  • Yes
  • No

Dependencies

none

Requirements

If you have made any visual changes to the GUI. Make sure you have:

  • Tested on iOS device
  • Tested on Android device
  • Tested on small-screen device (iPod Touch)
  • Tested on large-screen device (tablet)

@@ -484,7 +506,7 @@ function FloatingNavFixer(props: { navigation: NavigationBase }) {

// Retry once the keyboard drops or if we time out:
let handled = false
function retryNavigation() {
const retryNavigation: () => void = () => {
Copy link
Contributor

@swansontec swansontec Sep 3, 2025

Choose a reason for hiding this comment

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

const retryNavigation = (): void => { would be a simpler way to say the same thing

@@ -14,30 +7,23 @@ import {
asString,
asTuple
} from 'cleaners'
/* For our `handleGetPointerProps` handler: */
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this exception, and instead change the props variable to be called opts or something, inside handleGetPointerProps. The lint rule is picky about things called "props"

Comment on lines 59 to 60
// This rule doesn't reliably track TS interfaces on fn components...
// eslint-disable-next-line react/no-unused-prop-types
Copy link
Contributor

Choose a reason for hiding this comment

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

The problem is that you have export const SwipeChart: React.FC<Props> = (params: Props) => {, which should instead be export const SwipeChart: React.FC<Props> = props => {. Notice the variable rename, and the type removal. With this change in place, ESLint is fine.

@Jon-edge Jon-edge force-pushed the jon/rn79/swipe-chart branch from 6f76345 to b56b634 Compare September 3, 2025 20:23
@Jon-edge Jon-edge force-pushed the jon/rn79/swipe-chart branch 2 times, most recently from 0cd8c71 to b1b18d2 Compare September 4, 2025 00:02
@Jon-edge Jon-edge enabled auto-merge September 4, 2025 00:03
cursor[bot]

This comment was marked as outdated.

@Jon-edge Jon-edge disabled auto-merge September 4, 2025 00:10
@Jon-edge Jon-edge force-pushed the jon/rn79/swipe-chart branch from b1b18d2 to 1d602b3 Compare September 4, 2025 00:19
@Jon-edge Jon-edge enabled auto-merge September 4, 2025 00:20
cursor[bot]

This comment was marked as outdated.

@Jon-edge Jon-edge force-pushed the jon/rn79/swipe-chart branch 2 times, most recently from 45cbdde to f566740 Compare September 4, 2025 17:51
@Jon-edge Jon-edge force-pushed the jon/rn79/swipe-chart branch from f566740 to 8a02547 Compare September 4, 2025 17:52
@Jon-edge
Copy link
Collaborator Author

Jon-edge commented Sep 4, 2025

Bug: Notification Prop Removal Affects Layout

The hasNotifications prop was removed from SceneWrapper in CoinRankingDetailsScene and CoinRankingScene. This change appears unrelated to the chart migration and likely disables intended notification functionality and its associated layout area.

src/components/scenes/CoinRankingScene.tsx#L333-L337
src/components/scenes/CoinRankingDetailsScene.tsx#L634-L635
Fix in Cursor Fix in Web

This was an intentional cleanup. Scene already has no tabs, notifications were floating above some empty space.

Also, coinranking scenes are not showing any account-specific information, and there is a future ask to make it accessible outside of a logged in view.

Removing the notifications cleans up the experience where the user is trying to view a large amount of information on the screen at once.

@Jon-edge Jon-edge merged commit 465d995 into develop Sep 4, 2025
3 checks passed
@Jon-edge Jon-edge deleted the jon/rn79/swipe-chart branch September 4, 2025 18:13
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