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

Yuhsuan/scroll shadow #2447

Open
wants to merge 35 commits into
base: dev
Choose a base branch
from
Open

Yuhsuan/scroll shadow #2447

wants to merge 35 commits into from

Conversation

YuHsuan-Hwang
Copy link
Collaborator

@YuHsuan-Hwang YuHsuan-Hwang commented Dec 10, 2024

Description

Implemented features:

  1. Closes [Regression] Inconsistent dark theme background #2446 by fixing background regressions.
    1. The menu button dark theme color is changed to the overall background color.
    2. The z profile, histogram, and spectral line query widget dark theme background is changed to a darker color, consistent with other widgets.
    3. The undocked widget dark theme background is changed to a darker color, consistent with docked widgets and dialogs. Also adjusted catalog scatter/histogram plot dark theme background and exported png file dark theme background.
  2. Closes [v4-beta user feedback] always show the scroll bar for the input parameter pane in the image fitting widget #2166 by adding scroll shadows to several widgets, including:
    1. dialogs (preference dialog, save image panel in file browser dialog, contour dialog, vector dialog, image fitting dialog, online data query dialog, region dialog)
    2. settings widgets (spatial profiler settings, spectral profiler settings, histogram settings, stokes analysis settings, catalog settings)
    3. PV generator widget, animator widget, render config widget options, and z profile widget legend
  3. Closes Improve dark theme string color in the header information panel #2450: adjust the string colors in dark theme.
  4. Closes Improve consistency of table dark theme backgrounds #2451: adjust the blueprint table empty space background color and stats table background color in dark theme.

Note that we might also consider disabling the shadows when the scroll bar exists, and I prefer we create a separate issue for it if necessary.

Code changes:

  1. background related CSS changes
    1. Overwritten the menu button dark theme color.
    2. Avoided setting bp#-dark to elements which applies unnecessary additional styles in App.scss.
    3. For undocked widgets, changed the dark theme background color to $pt-dark-app-background-color in App.scss.
  2. Created a new component ScrollShadow which applies scroll shadows with a pure CSS approach. There are some disadvantages with the simple approach but most of them look minor:
    1. The shadow is implemented in the background. Input columns and buttons are displayed on top of it.
    2. Does not support custom background color. Currently, we only deal with two background colors (light and dark theme).
    3. In Safari and Firefox, we temporally see the shadows when scrolling to the end because of the interaction design.
  3. CSS changes: Changed the string colors in dark theme.
  4. CSS changes: Overwritten blueprint table dark theme background color. The tables in file browser and work space dialogs are set to a lighter color to display the border of the panel. Changed the stats table dark theme background colors.

Tested that:

  1. UI in the dark mode looks clear with the new background.
  2. The background colors in the light mode are unchanged.
  3. The shadows do not create double scroll bars.
  4. The shadows are responsive to the theme.
  5. Responsive UI in the animator and render config widget works as usual.

There are some failed e2e tests, to be investigated.

Checklist

For linked issues (if there are):

  • assignee and label added
  • GitHub Project estimate added

For the pull request:

  • reviewers and assignee added
  • GitHub Project estimate added
  • changelog updated / no changelog update needed
  • unit test added (for functions with no dependenies)
  • API documentation added (for public variables and methods in stores)

For dependencies:

  • e2e test passing / corresponding fix added / new e2e test created
  • protobuf version bumped / no protobuf version bumped needed
  • protobuf updated to the latest dev commit / no protobuf update needed
  • corresponding ICD test fix added (BackendService changed) / no ICD test fix needed (BackendService unchanged)
  • user manual prepared (for large new features)

@loveluthien loveluthien added awaiting code review For pull requests that require code review awaiting testing For pull requests that require testing awaiting code changes For pull requests that require code changes and removed awaiting code review For pull requests that require code review labels Dec 12, 2024
@YuHsuan-Hwang
Copy link
Collaborator Author

Update: Fixed #2450 and #2451. The details are updated in the description.

@YuHsuan-Hwang YuHsuan-Hwang added awaiting code review For pull requests that require code review and removed awaiting code changes For pull requests that require code changes labels Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting code review For pull requests that require code review awaiting testing For pull requests that require testing
Projects
None yet
2 participants