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

feat: use nivo responsive bar chart for progress and criticality bar #2639

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

meretp
Copy link
Contributor

@meretp meretp commented May 29, 2024

Summary of changes

To improve the progress bar this PR adds a new library nivo and introduces a ResponsiveBar.

Context and reason for change

fixes part of #2622

How can the changes be tested

Open OpossumUI and play around with the progress/criticality bar.

Note: Please review the guidelines for contributing to this repository.

@meretp meretp force-pushed the feat/improve-progress-bar branch 3 times, most recently from 2b8093a to 263b82b Compare May 29, 2024 08:42
@meretp meretp force-pushed the feat/improve-progress-bar branch 5 times, most recently from bd0f00b to 70a2cdb Compare June 24, 2024 11:11
@meretp meretp marked this pull request as ready for review June 24, 2024 11:46
@mstykow mstykow self-assigned this Jun 26, 2024
@mstykow mstykow force-pushed the feat/improve-progress-bar branch from 70a2cdb to 52bb95c Compare August 25, 2024 18:29
src/Frontend/Components/ProgressBar/ProgressBar.util.tsx Outdated Show resolved Hide resolved
src/Frontend/Components/ProgressBar/ProgressBar.util.tsx Outdated Show resolved Hide resolved
src/Frontend/Components/ProgressBar/ProgressBar.util.tsx Outdated Show resolved Hide resolved
margin: 2,
fontWeight: 500,
position: 'relative',
top: 100,
Copy link
Member

Choose a reason for hiding this comment

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

the positioning is not working consistently: depending on the bar's mode, the tooltip appears higher or lower below the cursor. could you find a way to make the positioning consistent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is still to do, but so far I couldn't find a way.

Copy link
Member

Choose a reason for hiding this comment

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

ok, i'll have a look

src/Frontend/Components/ProgressBar/ProgressBar.util.tsx Outdated Show resolved Hide resolved
src/Frontend/Components/ProgressBar/ProgressBar.tsx Outdated Show resolved Hide resolved
src/Frontend/Components/ProgressBar/ProgressBar.tsx Outdated Show resolved Hide resolved
src/Frontend/Components/ProgressBar/ProgressBar.tsx Outdated Show resolved Hide resolved
src/Frontend/Components/ProgressBar/ProgressBar.util.tsx Outdated Show resolved Hide resolved
@meretp meretp force-pushed the feat/improve-progress-bar branch from a9f8281 to 48494e3 Compare September 13, 2024 05:26
@meretp meretp force-pushed the feat/improve-progress-bar branch 2 times, most recently from 3cd1ce4 to f5db9c1 Compare September 20, 2024 05:37
src/Frontend/Components/ProgressBar/ProgressBar.util.tsx Outdated Show resolved Hide resolved
margin: 2,
fontWeight: 500,
position: 'relative',
top: 100,
Copy link
Member

Choose a reason for hiding this comment

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

ok, i'll have a look

@meretp meretp force-pushed the feat/improve-progress-bar branch from f5db9c1 to 793d773 Compare September 24, 2024 06:05
@mstykow mstykow force-pushed the feat/improve-progress-bar branch from 793d773 to 8932d9c Compare September 28, 2024 15:51
@mstykow mstykow force-pushed the feat/improve-progress-bar branch 2 times, most recently from 8dd902a to b4fece8 Compare January 2, 2025 16:11
@mstykow
Copy link
Member

mstykow commented Jan 2, 2025

The following issues need to be solved before this PR can merge:

  1. the tooltip position differs between tooltip modes
  2. when resizing the window (decrease width), the new bar chart does not resize itself thus causing x-overflow

@meretp meretp force-pushed the feat/improve-progress-bar branch from b4fece8 to a17f08b Compare January 20, 2025 07:13
@meretp
Copy link
Contributor Author

meretp commented Jan 20, 2025

Added some commits that fixes the second issue and updates nivo bar - there has been a new release in the meantime. Also I updated the diff to use the ariaLabel from ResponsiveBar.

meretp and others added 6 commits January 20, 2025 08:16
Signed-off-by: Meret Behrens <meret.behrens@tngtech.com>
Signed-off-by: Maxim Stykow <maxim.stykow@tngtech.com>
Signed-off-by: Meret Behrens <meret.behrens@tngtech.com>
There are still some issues:
1. the position differs between tooltip modes
2. when resizing the window down, the new bar chart does not resize itself thus causing x-overflow

Signed-off-by: Maxim Stykow <maxim.stykow@tngtech.com>
@meretp meretp force-pushed the feat/improve-progress-bar branch from a17f08b to 8df5617 Compare January 20, 2025 07:19
@meretp
Copy link
Contributor Author

meretp commented Jan 20, 2025

To fix the first issue the only idea I could come up with is to use sepearate CSS styling like

transformProgress: { transform: 'translate(0px, 168px)' } as CSSProperties,
transformCritical: { transform: 'translate(0px, 144px)' } as CSSProperties,

to transform the tooltips separately. This does not feel like a stable solution so I only add this as a comment not commit yet, what do you think?

@meretp
Copy link
Contributor Author

meretp commented Jan 20, 2025

Have to take a look at why the unit tests are failing now

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.

2 participants