Skip to content

Conversation

@Artem-Semenov-dev
Copy link
Contributor

This PR implements the component purposed for the chart drawing.
An example of the chart view:
image

@Vladyslav-Kuksiuk
Copy link

It's better to add labels to the axes, like in Google Finance.

image

Copy link

@Vladyslav-Kuksiuk Vladyslav-Kuksiuk left a comment

Choose a reason for hiding this comment

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

@Artem-Semenov-dev please see my comment.

Comment on lines +100 to +103
* @param inMin The minimum value of the original range
* @param inMax The maximum value of the original range
* @param outMin The minimum value of the target range
* @param outMax The maximum value of the target range

Choose a reason for hiding this comment

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

Suggested change
* @param inMin The minimum value of the original range
* @param inMax The maximum value of the original range
* @param outMin The minimum value of the target range
* @param outMax The maximum value of the target range
* @param inMin the minimum value of the original range
* @param inMax the maximum value of the original range
* @param outMin the minimum value of the target range
* @param outMax the maximum value of the target range

Also, it might be better to name the parameters as originalMin and targetMin.

@Artem-Semenov-dev Artem-Semenov-dev marked this pull request as ready for review July 31, 2023 13:20
@Artem-Semenov-dev Artem-Semenov-dev requested a review from armiol July 31, 2023 13:20
@armiol armiol assigned armiol and unassigned Artem-Semenov-dev Aug 2, 2023
@armiol armiol removed their request for review August 2, 2023 12:34
@armiol
Copy link
Contributor

armiol commented Aug 2, 2023

@Artem-Semenov-dev I am reassigning this PR to myself now. Going to finish it once I have an opportunity.

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.

4 participants