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

feat: graph events #84

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: graph events #84

wants to merge 2 commits into from

Conversation

PeKne
Copy link

@PeKne PeKne commented Jul 17, 2023

Description

This pull request introduces support for graph events in the application. The main goal is to enhance the user experience by providing the ability to render events in the line graph and highlight important interactions.

To achieve this, the AnimatedLineGraph component accepts four new props events, EventComponent, EventTooltipComponent and onEventHover.

  • events is an array of objects that represents each individual interaction and its associated metadata. Each object in this array includes a date property, which is used to calculate the coordinates of the event on the graph.
  • EventComponent is a component that receives the coordinates and metadata of each event and renders it within the graph.
  • EventTooltipComponent optional element that is rendered when hovering over an EventComponent element.
  • onEventHover is a callback that triggers hovering over an EventComponent allowing for additional actions.

Demo

Screen.Recording.2023-07-17.at.7.46.52.mov

Support for graph events added.
### `EventTooltipComponent`
An additional event component that is rendered if the `SelectionDot` overlaps an `Event`.

### `onEventHover`

This comment was marked as outdated.

Copy link
Author

Choose a reason for hiding this comment

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

The library does not work with any tap gestures so far. The only user interaction so far is created by the pan gesture which is also the gesture that enables the new event functionality of this PR. I would prefer to not add this functionality to not make the PR too big. The onPressEvent could be added to the follow-up PR if needed.

### `EventComponent`
A component that is used to render an event.

### `EventTooltipComponent`
Copy link
Contributor

Choose a reason for hiding this comment

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

Please create some default component EventTooltip and link it here as example (same asi SelectionDot)

Copy link
Author

Choose a reason for hiding this comment

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

Default component created in the fixup 5ff9fd7.


### `EventComponent`
A component that is used to render an event.
Copy link
Contributor

Choose a reason for hiding this comment

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

Link EventComponent file here as example (same as SelectionDot)

Copy link
Author

Choose a reason for hiding this comment

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

Linked in the fixup 5ff9fd7.

README.md Outdated

Example:

<img src="./img/events.png" align="right" height="200" />
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Can you make default color of event different that line? Maybe some kind of simple fade out gradient of dot or something like that will make it really nice.

Copy link
Author

Choose a reason for hiding this comment

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

The style of the default event component was adjusted to appear more contrasting. 5ff9fd7

onEventHover?: () => void
) => {
const [activeEventIndex, setActiveEventIndex] = useState<number | null>(null)
const handleDisplayEventTooltip = (
Copy link
Contributor

Choose a reason for hiding this comment

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

useCallback

Copy link
Author

Choose a reason for hiding this comment

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

Fixed in 5ff9fd7.

@mrousavy
Copy link
Member

Oh wow, this is pretty cool. CC @chrispader

}

export const LineGraph = React.memo(LineGraphImpl)
export const LineGraph = React.memo(LineGraphImpl) as typeof LineGraphImpl
Copy link
Member

Choose a reason for hiding this comment

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

Why? This should be typed

@chrispader
Copy link
Member

I'll try to review this during this week! 👍

@PeKne
Copy link
Author

PeKne commented Oct 30, 2023

@chrispader Any progress with the review? 🙏🙂

@ikryvorotenko
Copy link

hey guys, any updates on this?

@chrispader
Copy link
Member

Will look into this tmrw!

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.

5 participants