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

SY-1221 Temporarily Focus a Layout #834

Merged
merged 13 commits into from
Sep 26, 2024
Merged

Conversation

emilbon99
Copy link
Contributor

@emilbon99 emilbon99 commented Sep 21, 2024

Feature Pull Request Template

Key Information

Description

Allows the user to temporarily focus a layout to make it occupy a larger portion of the screen.

Basic Readiness Checklist

  • I have performed a self-review of my code.
  • I have added relevant tests to cover the changes to CI.
  • I have updated user facing documentation accordingly.
  • I have verified code coverage targets are met.

Migrations

  • Console - I have ensured that previous versions of stored data structures are
    properly migrated to new formats.
  • Server - I have ensured that previous versions of stored data structures are
    properly migrated to new formats.

Additional Notes

  • These changes deal with concurrency.
  • These changes affect UI.

Manual QA Additions

  • I have updated the Release Candidate template
    with necessary manual QA steps to test my change.

Breaking Changes

Please list any breaking changes to public or internal packages.

Reviwer Checklist

  • Sufficient test coverage of new additions.
  • Verified all steps in the Readiness checklists.
  • UI changes have been tested.
  • Style and formatting is consistent.
  • Reviewed any relevant changes to concurrent code for safety.
  • Sufficient comments and clarity of code.
  • Any additional documentation necessary for feature is provided and has been reviewed for clarity.

@codecov-commenter
Copy link

codecov-commenter commented Sep 21, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 40.08264% with 290 lines in your changes missing coverage. Please review.

Project coverage is 46.33%. Comparing base (3c6a44d) to head (c6ad614).
Report is 21 commits behind head on rc.

Files with missing lines Patch % Lines
pluto/src/vis/diagram/Diagram.tsx 0.00% 44 Missing ⚠️
pluto/src/vis/lineplot/Legend.tsx 0.00% 37 Missing ⚠️
pluto/src/mosaic/Mosaic.tsx 23.68% 29 Missing ⚠️
pluto/src/vis/legend/Simple.tsx 0.00% 29 Missing ⚠️
pluto/src/vis/lineplot/LinePlot.tsx 0.00% 19 Missing ⚠️
pluto/src/modal/Modal.tsx 0.00% 18 Missing ⚠️
pluto/src/vis/grid/grid.ts 71.69% 15 Missing ⚠️
pluto/src/menu/Item.tsx 12.50% 14 Missing ⚠️
pluto/src/mosaic/tree.ts 74.54% 14 Missing ⚠️
pluto/src/vis/lineplot/aether/axis.ts 0.00% 11 Missing ⚠️
... and 18 more

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##               rc     #834      +/-   ##
==========================================
- Coverage   46.34%   46.33%   -0.01%     
==========================================
  Files        1088     1089       +1     
  Lines       67885    68050     +165     
  Branches     3512     3503       -9     
==========================================
+ Hits        31462    31532      +70     
- Misses      35372    35472     +100     
+ Partials     1051     1046       -5     
Flag Coverage Δ
aspen 50.40% <ø> (-0.07%) ⬇️
cesium 75.53% <ø> (ø)
clientpy 86.04% <ø> (ø)
clientts 61.36% <ø> (+0.01%) ⬆️
drift 26.27% <0.00%> (-0.03%) ⬇️
pluto 35.42% <40.24%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@pjdotson pjdotson left a comment

Choose a reason for hiding this comment

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

Three things:

First, a minor problem - when you select on the tab selector bar to the right of all existing tabs, a context menu with only hard reload does not appear.

Second, I'm getting a bunch of errors in the devtools console when I play with windows and move them around (listed down below.

Third, I opened a bunch of tabs and then spammed cmd + W quickly (like three times). This caused the app to close before all of the tabs that were existing closed.

Warning: React does not recognize the borderShade prop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercase bordershade instead. If you accidentally passed it from a parent component, remove it from the DOM element

Warning: React does not recognize the endIcon prop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercase endicon instead. If you accidentally passed it from a parent component, remove it from the DOM element.
button

Warning: Received true for a non-boolean attribute visible.

If you want to write it to the DOM, pass a string instead: visible="true" or visible={value.toString()}.

Warning: Received false for a non-boolean attribute focused.

If you want to write it to the DOM, pass a string instead: focused="false" or focused={value.toString()}.

If you used to conditionally omit it with focused={condition && value}, pass focused={condition ? value : undefined} instead.

drift/src/sync.ts Show resolved Hide resolved
pluto/src/theming/core/theme.ts Outdated Show resolved Hide resolved
pluto/src/mosaic/Mosaic.tsx Outdated Show resolved Hide resolved
pluto/src/vis/canvas/Canvas.css Show resolved Hide resolved
console/src/vis/Canvas.css Show resolved Hide resolved
@emilbon99
Copy link
Contributor Author

@pjdotson I fixed the hard reload item. Issues with unrecognized props have been fixed on #830

Copy link
Contributor

@pjdotson pjdotson left a comment

Choose a reason for hiding this comment

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

Pressing Cmd + R to rename a tab does not seem to work. Other than that it looks good to me.

@emilbon99 emilbon99 merged commit 674825d into rc Sep 26, 2024
27 of 28 checks passed
@emilbon99 emilbon99 deleted the sy-1221-temporarily-focus-a-layout branch September 26, 2024 21:03
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