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

fix some viewer bugs #2371

Merged
merged 1 commit into from
Oct 5, 2023
Merged

fix some viewer bugs #2371

merged 1 commit into from
Oct 5, 2023

Conversation

ada-x64
Copy link
Contributor

@ada-x64 ada-x64 commented Sep 28, 2023

This PR fixes a few small bugs.

  • The perspective-datagrid-plugin was throwing some annoying errors when you switched between plugins. This was because we are registering an event listener on the perspective-viewer element, and it is never being unregistered. The only way we can unregister the event listener is to add a deactivate method to the API where we can clean up the listeners. For now, I added a quick hack.
  • Fixed some visual bugs regarding borders on the side panels. The top border should no longer shrink and the column settings panel should now go beneath the settings panel when resizing.

Copy link
Member

@texodus texodus left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Looks good!

@@ -10,6 +10,10 @@
// ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃
// ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛

:host {
--settings-panel-z-index: 10;
Copy link
Member

Choose a reason for hiding this comment

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

IMO we want to save settings for theme-able properties, not just constants. Perhaps we can add a special namespace for "private" variables.

@texodus texodus merged commit 542c37a into finos:master Oct 5, 2023
12 checks passed
@texodus texodus added the bug Concrete, reproducible bugs label Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Concrete, reproducible bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants