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

Added color vision deficiency options to Plotly visualizations #910

Closed

Conversation

TejasNangru
Copy link

@TejasNangru TejasNangru commented Feb 17, 2025

Description

This PR introduces Color Vision Deficiency (CVD) accessibility options to Plotly visualizations in the ReactiveChart.vue component. The following changes have been made:

  1. Integrated a dynamic dropdown menu using Quasar's QSelect component.
  2. Integrated multiple color palettes for different types of color vision deficiencies (Deuteranopia, Protanopia, Tritanopia) in the chart visualizations.
  3. Added a ModeBar button for toggling between different color palettes.
  4. Ensured compatibility with Plotly’s Plotly.react for rendering charts dynamically.
  5. The user preferences for CVD modes are intended to be stored using idb-keyval for persistence.
  6. This update enhances the accessibility of the visualizations for users with color vision impairments, contributing to a more inclusive and usable platform. The functionality of dynamically rendering charts with user-selected color modes has been preserved.

Resolves Issue #898.

Related issue

#898

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@TejasNangru
Copy link
Author

@dpgiakatos sir please review my changes.

@dpgiakatos dpgiakatos self-requested a review February 17, 2025 14:26
Copy link
Member

@dpgiakatos dpgiakatos left a comment

Choose a reason for hiding this comment

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

This PR should not have 165 deletions. I do not understand why this PR deletes code that is being used in the codebase and should not be modified for this feature. More specifically, the props have been modified (they shouldn't), the emit functions have been deleted, object declarations have been deleted, and functions have been deleted. On the contrary, there are small changes introducing the new feature. This concludes that the modifications were made by LLM. This PR should not be merged.

@TejasNangru
Copy link
Author

@dpgiakatos sir i have tried to modify the code, please have a look.

@dpgiakatos dpgiakatos self-requested a review February 17, 2025 15:35
Copy link
Member

@dpgiakatos dpgiakatos left a comment

Choose a reason for hiding this comment

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

This PR does not address the comments made in the first review. Additionally, the code remains largely unchanged, with only minor differences, such as added comments, which do not impact the fact that an important part of the code has been removed. Therefore, I am closing this PR. The contributor may want to consider finding another issue that is more beginner-friendly in order to get familiar with the codebase.

@dpgiakatos dpgiakatos closed this Feb 17, 2025
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