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

Feature/ant5 #626

Merged
merged 8 commits into from
Feb 18, 2025
Merged

Feature/ant5 #626

merged 8 commits into from
Feb 18, 2025

Conversation

interim17
Copy link
Contributor

@interim17 interim17 commented Feb 3, 2025

Time estimate or Size

?

Huge change set, but everything has been reviewed once already.
I would say run the brunch, look for any styling issues?

Problem

Migrating ant 4 -> 5

Solution

Lot's of changes, happy to talk through previous PRs or any specific questions.

The broad overview:

  • bumped antd, typescript and made some associated config changes
  • removed some dead code
  • made lots of little changes to style sheets and component props because
  • added theming via antd ConfigProvider
  • added styled-components, integrated it into theming and used it to style a few components that are hard to target, or require a lot of styling overrides

#609)

* bump antd and typescript, and update build tools and components to work with new versions

* consolidate ts.config
* bump antd and typescript, and update build tools and components to work with new versions

* consolidate ts.config

* replace ant-vars.less with ConfigProvider

* semantic color variables names for config provider

* align css var colors and theme color variables, divide light and dark theme colors

* consolidate theme variables

* further consolidate theme variables

* styled-components.dropdown menu items (#616)

* use styled-component to streamline appearance of dropdown menu items

* prevent ant from overridding text color during hover/focus on dropdown items

* trim unneedd props from baseStyles on dropdown items

* restructure colors and theming to provide for styled-components use of themeColors

* remove unused nav button drop down item variety

* don't open drop downs by default

* remove unused imports

* make selector more specific and remove !important tag

* change focus to focus-visible in selectors and typing for styled-components theme

* only export semantic and component colors from theme
* bump antd and typescript, and update build tools and components to work with new versions

* consolidate ts.config

* replace ant-vars.less with ConfigProvider

* semantic color variables names for config provider

* align css var colors and theme color variables, divide light and dark theme colors

* consolidate theme variables

* styling fixes for antd5

* fix record icon position

* pass onClick into dropdown menu items

* fix meta data panel font sizes

* fix colorpicker alignment offset

* remove unused commented style rule

* remove unused themeConfig

* bump viewer version

* replace clickHandler with onClick

* a few more ant migration style fixes

* restore className on dropdown render div

* remove unused import
* bump antd and typescript, and update build tools and components to work with new versions

* consolidate ts.config

* replace ant-vars.less with ConfigProvider

* semantic color variables names for config provider

* align css var colors and theme color variables, divide light and dark theme colors

* consolidate theme variables

* styling fixes for antd5

* fix record icon position

* pass onClick into dropdown menu items

* fix meta data panel font sizes

* fix colorpicker alignment offset

* remove unused commented style rule

* remove unused themeConfig

* apply styling fixes and use new CustomButton component to streamline button styling

* bump viewer version

* replace clickHandler with onClick

* a few more ant migration style fixes

* restore className on dropdown render div

* remove duplicate import

* remove unused css vars

* remove unused import

* use plain const for tooltip text on custom buttons

* fix focus styling
Copy link

github-actions bot commented Feb 3, 2025

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
67.47% (+0.22% 🔼)
728/1079
🟡 Branches
67.07% (+0.21% 🔼)
110/164
🔴 Functions
35.9% (-0.47% 🔻)
98/273
🟡 Lines
65.56% (-0.11% 🔻)
651/993

Test suite run success

137 tests passing in 8 suites.

Report generated by 🧪jest coverage report action from 12dd053

@interim17 interim17 marked this pull request as ready for review February 3, 2025 19:09
@interim17 interim17 requested a review from a team as a code owner February 3, 2025 19:09
@interim17 interim17 requested review from toloudis and frasercl and removed request for a team February 3, 2025 19:09
@toloudis toloudis requested review from rugeli and removed request for toloudis February 3, 2025 20:49
Copy link
Contributor

@rugeli rugeli left a comment

Choose a reason for hiding this comment

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

I have clicked around on the branch and the styling looked good, everything functional as expected!

Super minor and non-blocking, but thought I'd mention it: when loading the blood plasma model, the v.1.0 tag shows up first in the title and stays grayed out the whole time.

Screenshot 2025-02-04 at 3 24 54 PM

@frasercl
Copy link
Contributor

frasercl commented Feb 6, 2025

Poking around the UI, I noticed a few potentially problematic visual changes:

  1. On the home page, hovering over the images in the example model cards causes the margin between the images and the description to shrink:
    image
  2. The hover state of dropdown items has a new background, rather than a text color change, which reveals that the text is vertically off-center. (This may be intentional, and the centered-ness may be acceptable.)
    image
  3. The "help" dropdown is missing an outline that is present in the deployed version:
    image
  4. Plots have a new border radius and margin, which reveals a gap between the header ("Plots") and the first plot in the list which wasn't there before:
    image
  5. Like Ruge said, the version tag in the title bar is barely readable.

Up to your discretion whether or not to address any one of these - I think only 1 and maybe 5 are true problems.

@interim17
Copy link
Contributor Author

@frasercl and @rugeli thanks for the comments

  • homepage images: fixed
  • help button border: fixed
  • version tag colors: fixed
  • dropdown hover items: background color change is correct, vertical text alignment fixed although this relates to an overpass font issue I'd like a more long term solution for
  • plots: top margin is correct, border-radius is correct re convo w ux, I think some work on plot formatting is worth doing soon but out of scope

@interim17 interim17 merged commit 2aa31a3 into main Feb 18, 2025
6 checks passed
@interim17 interim17 deleted the feature/ant5 branch February 18, 2025 05:36
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