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

ChartRedraw Fix #152 #153

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

ChartRedraw Fix #152 #153

wants to merge 5 commits into from

Conversation

laurenic0l
Copy link
Contributor

  • Replaced List.of() with Arrays.asList(new Object[]) to allow mixed-type arguments (e.g., String and Integer) in chart data rows.

This fix resolves issue #152 related to List.of() incompatibility with heterogeneous data types.

- Replaced `List.of()` with `Arrays.asList(new Object[])` to allow
mixed-type arguments (e.g., String and Integer) in chart data rows.
- Updated header and dynamic row addition logic to ensure compatibility
with mixed data types.

This fix resolves issue #152 related to `List.of()` incompatibility with
heterogeneous data types.
Copy link
Member

@hyyan hyyan left a comment

Choose a reason for hiding this comment

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

@laurenic0l This is not the appropriate way to resolve this issue. Please refactor the code to specify the columns and data arrays, similar to what you did in ChartView.java.

@laurenic0l laurenic0l requested a review from hyyan January 14, 2025 19:39
@laurenic0l
Copy link
Contributor Author

@jturfanbasis ready for review

Copy link

@jturfanbasis jturfanbasis left a comment

Choose a reason for hiding this comment

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

Thank you for the changes! While testing, I noticed that the "Redraw Chart" button's layout feels cluttered due to the lack of spacing between the button and the input fields. Can you fix it, please?
image

@laurenic0l
Copy link
Contributor Author

@jturfanbasis Thanks for catching that, I realized I didn't push the css file. Let me know if anything else!

jturfanbasis
jturfanbasis previously approved these changes Jan 14, 2025
Copy link
Member

@hyyan hyyan left a comment

Choose a reason for hiding this comment

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

@laurenic0l, can you please wrap the inputs in a flex container and make that them wrap for smaller screens.

@laurenic0l
Copy link
Contributor Author

@hyyan done!

Copy link

@jturfanbasis jturfanbasis left a comment

Choose a reason for hiding this comment

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

When the user resizes the browser window, there is still no scroll bar.

You can find related bug report. #145

The application limits values to 2,147,483,647, which is the maximum for a 32-bit integer. However, when a user enters a value exceeding this limit, the application does not display a user-friendly error message. This should be improved to inform users why their input is invalid in a clear and understandable way.

image
image

@hyyan
Copy link
Member

hyyan commented Jan 24, 2025

The application limits values to 2,147,483,647, which is the maximum for a 32-bit integer. However, when a user enters a value exceeding this limit, the application does not display a user-friendly error message. This should be improved to inform users why their input is invalid in a clear and understandable way.

Hey @jturfanbasis, please keep in mind that these are just demos. Our main goal is to introduce code that runs without any exceptions and should be as simple as possible to demonstrate the sample’s purpose. Validations should be added when they make sense, but guarding against edge cases is off-topic and outside the scope of these demos. Please don’t hesitate to report any details, and we’ll assess them case by case. We really appreciate your effort!

@jturfanbasis
Copy link

The application limits values to 2,147,483,647, which is the maximum for a 32-bit integer. However, when a user enters a value exceeding this limit, the application does not display a user-friendly error message. This should be improved to inform users why their input is invalid in a clear and understandable way.

Hey @jturfanbasis, please keep in mind that these are just demos. Our main goal is to introduce code that runs without any exceptions and should be as simple as possible to demonstrate the sample’s purpose. Validations should be added when they make sense, but guarding against edge cases is off-topic and outside the scope of these demos. Please don’t hesitate to report any details, and we’ll assess them case by case. We really appreciate your effort!

Thank you Hyyan!

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