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(webapp): update timeline appearance and refactor flot plugins #1323

Merged
merged 12 commits into from
Aug 12, 2022

Conversation

pavelpashkovsky
Copy link
Contributor

@pavelpashkovsky pavelpashkovsky commented Jul 29, 2022

Short summary on this PR
We don't edit any code in Flot anymore. But there are cases when new functionality based on some built-in plugins. In case of upgrading time selection functionality I extracted react-flot/flot/jquery.flot.selection.min into a file ./TimelineChartSelection, where I customised existing logic in accordance with task requirements. It's important to note, that there are 2 types of timeline selection: double and single. All customisation was applied only for single type of selection. Double works in old fashion way (using built-in Flot selection, not plugin-built one).

image

image


I also extended in a bit TimelineChartWrapper API. Now you can set colors for selection boundaries and overlay. Adding colors depending on color mode is in progress

image

@pavelpashkovsky pavelpashkovsky changed the title feat: update timeline appearance/ flot pugins refactor feat(webapp): update timeline appearance/ flot pugins refactor [WIP] Jul 29, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jul 29, 2022

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
webapp/public/assets/app.js 411.17 KB (+0.27% 🔺) 8.3 s (+0.27% 🔺) 3.5 s (+9.32% 🔺) 11.8 s
webapp/public/assets/app.css 14.01 KB (0%) 281 ms (0%) 0 ms (+100% 🔺) 281 ms
webapp/public/assets/styles.css 9.82 KB (0%) 197 ms (0%) 0 ms (+100% 🔺) 197 ms
packages/pyroscope-flamegraph/dist/index.js 90.13 KB (0%) 1.9 s (0%) 1.3 s (-6.49% 🔽) 3.1 s
packages/pyroscope-flamegraph/dist/index.node.js 81.46 KB (0%) 1.7 s (0%) 680 ms (+27.98% 🔺) 2.4 s
packages/pyroscope-flamegraph/dist/index.css 5.83 KB (0%) 117 ms (0%) 0 ms (+100% 🔺) 117 ms

@codecov
Copy link

codecov bot commented Jul 29, 2022

Codecov Report

Merging #1323 (227a088) into main (eab8ef9) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1323   +/-   ##
=======================================
  Coverage   67.55%   67.55%           
=======================================
  Files         123      123           
  Lines        4040     4040           
  Branches      930      930           
=======================================
  Hits         2729     2729           
  Misses       1307     1307           
  Partials        4        4           
Impacted Files Coverage Δ
...javascript/pages/tagExplorer/components/Legend.tsx 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@pavelpashkovsky
Copy link
Contributor Author

/create-server

@pavelpashkovsky
Copy link
Contributor Author

looks like selection boundaries and overlay are too much pale.

@pavelpashkovsky
Copy link
Contributor Author

@Rperry2174, I can be wrong, but I've just remembered that you wanted to remove these captions several months ago? Should I do this?

image

from: leftFrom,
to: leftUntil,
color: leftColor,
overlayColor: leftColor.alpha(0.3),
Copy link
Contributor

Choose a reason for hiding this comment

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

What does overLayColor correspond to? is it this
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Rperry2174 you are right, it's visible semi transparent area between selection boundaries, you can set it's color by changing overlayColor prop

@Rperry2174
Copy link
Contributor

@Rperry2174, I can be wrong, but I've just remembered that you wanted to remove these captions several months ago? Should I do this?

Yeah we can remove these now.

Given the current way that the code is structured what would be the steps to be able to control the selection like this?
time_selector_mockup_00_3

@eh-am
Copy link
Collaborator

eh-am commented Aug 2, 2022

/create-server

@Rperry2174
Copy link
Contributor

/create-server

@pavelpashkovsky
Copy link
Contributor Author

/create-server

2 similar comments
@eh-am
Copy link
Collaborator

eh-am commented Aug 8, 2022

/create-server

@Rperry2174
Copy link
Contributor

/create-server

@eh-am
Copy link
Collaborator

eh-am commented Aug 9, 2022

/create-server

@Rperry2174 Rperry2174 marked this pull request as ready for review August 9, 2022 18:58
@Rperry2174
Copy link
Contributor

Rperry2174 commented Aug 9, 2022

almost there! I definitely like these selectors better.. Can we make this cursor an "open" hand:
image

@pavelpashkovsky
Copy link
Contributor Author

@Rperry2174, I changed resize cursor to open hand, please re/create-server

@Rperry2174
Copy link
Contributor

/create-server

Copy link
Contributor

@Rperry2174 Rperry2174 left a comment

Choose a reason for hiding this comment

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

LGTM! Nice work @pavelpashkovsky

@Rperry2174 Rperry2174 changed the title feat(webapp): update timeline appearance/ flot pugins refactor [WIP] feat(webapp): update timeline appearance and refactor flot plugins Aug 12, 2022
@Rperry2174 Rperry2174 merged commit 9393449 into main Aug 12, 2022
@Rperry2174 Rperry2174 deleted the timeline-refactor branch August 12, 2022 01:46
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.

3 participants