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

Improvements to reduce repainting #5876

Merged
merged 35 commits into from
Mar 17, 2023
Merged

Improvements to reduce repainting #5876

merged 35 commits into from
Mar 17, 2023

Conversation

charlesh88
Copy link
Contributor

@charlesh88 charlesh88 commented Oct 14, 2022

Closes #5247. Test instructions not yet included in the #5247 - will update there after we determine the best way to actually test these changes.

Describe your changes:

  • Removes a CSS filter hover effect that seemed to cause a lot of repaints; replaced with a box-shadow effect in buttons and simple background color changes in other elements.
  • Changes usage of transition: all to use specific properties instead via new mixin transition.
  • Convert __pane-main to use absolute positioning to isolate object view and Time Conductor from page reflowing.

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Is this change backwards compatible? For example, developers won't need to change how they are calling the API or how they've extended core plugins such as Tables or Plots.

Author Checklist

  • Changes address original issue?
  • Tests included and/or updated with changes? N/A no functional changes.
  • Command line build passes?
  • Has this been smoke tested?
  • Testing instructions included in associated issue OR is this a dependency/testcase change?

Reviewer Checklist

  • Changes appear to address issue?
  • Reviewer has tested changes by following the provided instructions?
  • Changes appear not to be breaking changes?
  • Appropriate automated tests included?
  • Code style and in-line documentation are appropriate?
  • Has associated issue been labelled unverified? (only applicable if this PR closes the issue)
  • Has associated issue been labelled bug? (only applicable if this PR is for a bug fix)

- WIP! Comments and radioactive trace colors added.
- Added comments, many items to be refactored.
- Related fixes for search/filter inputs in telemetry tables.
- Changes to reduce paints and improve performance.
- Remove/refactor `filter`-based hover styles.
- New `cControlHov` mixin that uses `box-shadow` instead of `filter`.
- Refactor `transition: all` to use more specific properties.
- New `transition` mixin for above.
- Cleanup; removed unused hover and style definitions.
- Cleanup.
@codecov
Copy link

codecov bot commented Oct 14, 2022

Codecov Report

Merging #5876 (83ff0dc) into master (8831b75) will decrease coverage by 0.79%.
The diff coverage is n/a.

❗ Current head 83ff0dc differs from pull request most recent head b1c373f. Consider uploading reports for the commit b1c373f to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5876      +/-   ##
==========================================
- Coverage   54.81%   54.03%   -0.79%     
==========================================
  Files         626      626              
  Lines       26630    26630              
  Branches     2405     2405              
==========================================
- Hits        14598    14390     -208     
- Misses      11370    11577     +207     
- Partials      662      663       +1     
Flag Coverage Δ *Carryforward flag
e2e-ci 39.35% <ø> (ø) Carriedforward from 8831b75
e2e-full 51.08% <ø> (ø) Carriedforward from 8831b75
e2e-stable 50.65% <ø> (-4.77%) ⬇️
unit 49.18% <ø> (+0.02%) ⬆️

*This pull request uses carry forward flags. Click here to find out more.

see 32 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8831b75...b1c373f. Read the comment docs.

charlesh88 and others added 2 commits October 14, 2022 16:39
- Convert `__pane-main` to use absolute positioning to isolate object view and Time Conductor from page reflowing.
Copy link
Contributor

@michaelrogers michaelrogers left a comment

Choose a reason for hiding this comment

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

These changes look good to me!

@michaelrogers michaelrogers enabled auto-merge (squash) December 19, 2022 22:13
@akhenry
Copy link
Contributor

akhenry commented Jan 9, 2023

@michaelrogers to address any broken tests and merge.

@davetsay davetsay added this to the Target:2.2.0 milestone Jan 30, 2023
@davetsay
Copy link
Contributor

@michaelrogers to fix tests and merge

@ozyx
Copy link
Contributor

ozyx commented Jan 31, 2023

From the failed e2e tests traces, it looks like the "Edit & Save" button is disappeared?

@unlikelyzero
Copy link
Collaborator

This one needs some additional work. We should move it back into draft

@michaelrogers
Copy link
Contributor

Resolved Failing Tests

  • Browse bar hidden due to absolute positioning: resolved by position:relative and adding an toolbar height to top while in edit mode
  • Collapsed inspector that was blocking the save button visibility
  • Autoscale test: appears to be failing due to orange border applied to overlay plot

@unlikelyzero unlikelyzero enabled auto-merge (squash) March 16, 2023 20:49
@akhenry akhenry added the sim-3 label Mar 17, 2023
@unlikelyzero unlikelyzero merged commit a3c5450 into master Mar 17, 2023
@unlikelyzero unlikelyzero deleted the fix-repaint-5247 branch March 17, 2023 20:07
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.

[Performance] Inspector Repaints in Local Clock and Remote Clock Time
6 participants