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

Graph feedback #716

Merged
merged 16 commits into from
Jul 12, 2023
Merged

Graph feedback #716

merged 16 commits into from
Jul 12, 2023

Conversation

lubej
Copy link
Collaborator

@lubej lubej commented Jul 11, 2023

  1. (Desktop) ParaTime should directly navigate when zoomed out.
  2. When zoomed in the user should be able to see that status of the ParaTime.
  3. When zoomed into a specific ParaTime any unrelated ParaTimes should have 50% opacity.
  4. The outer stroke should be display on top of the graph content.
  5. Reduce opacity on mobile tooltip.
  6. Fix ‘Coming soon’ label displayed on mobile.
  7. Shadows cut off on testnet graph.
  8. Add selected network label.

@github-actions
Copy link

github-actions bot commented Jul 11, 2023

Deployed to Cloudflare Pages

Latest commit: 70ec9026374d102cb02759052791ab8e69295c83
Status:✅ Deploy successful!
Preview URL: https://88e88999.oasis-explorer.pages.dev

@lubej lubej marked this pull request as ready for review July 11, 2023 17:02
@lubej lubej requested review from buberdds, lukaw3d and csillag and removed request for buberdds July 11, 2023 17:02
@lubej lubej force-pushed the ml/graph-feedback branch 4 times, most recently from 2e130b9 to 13505f5 Compare July 12, 2023 08:52
[isLayerDisabled],
)

const enabledLayers: Layer[] = useMemo(
Copy link
Contributor

@buberdds buberdds Jul 12, 2023

Choose a reason for hiding this comment

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

Enabled layers are defined in RouteUtils.getEnabledLayersForNetwork(network) why we need to find out what layers are enabled based on disabledMap which was created from getEnabledLayersForNetwork?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is used on multiple places in the graph, and it just an abstraction, to not repeat the function call.

Copy link
Contributor

Choose a reason for hiding this comment

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

but why we need Object.keys(disabledMap).filter(layer => !disabledMap[layer as Layer]).map(layer => layer as Layer), to get enabledLayers when RouteUtils.getEnabledLayersForNetwork(network) returns it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point. Will simplify.

@buberdds
Copy link
Contributor

buberdds commented Jul 12, 2023

  1. Graph position if wrong. It's moved to right and bottom and it overlaps "Selected Network"
  2. Emerald is broken on mobile (it should work like Cipher)

[Layer.cipher]: isLayerDisabled(Layer.cipher),
[Layer.sapphire]: isLayerDisabled(Layer.sapphire),
}
const isLayerDisabled = useCallback(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why we need to wrap getEnabledLayersForNetwork with useCallback when disabledMap is going to be memoized?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Due to network dependency. Status call depends on disabledMap, which I want to trigger only when network changes.

@lubej lubej enabled auto-merge July 12, 2023 16:15
@lubej lubej merged commit c3d9cf2 into master Jul 12, 2023
7 checks passed
@lubej lubej deleted the ml/graph-feedback branch July 12, 2023 16:17
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.

None yet

2 participants