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

Add error message to "Data Series" section #5152

Closed
wants to merge 3 commits into from

Conversation

julieg18
Copy link
Contributor

@julieg18 julieg18 commented Dec 23, 2023

Demo

Screen.Recording.2024-01-02.at.11.59.09.AM.mov

To Do

  • Fix bug where errors don't show unless dvc.yaml changes in session
  • Tests

Part of #5087

@julieg18 julieg18 added the product PR that affects product label Dec 23, 2023
@julieg18 julieg18 self-assigned this Dec 23, 2023
@julieg18 julieg18 changed the title Add error message to "Data Series" section Add error message to "Data Series" section Dec 23, 2023
import { TemplatePlots } from './TemplatePlots'
import { PlotsContainer } from '../PlotsContainer'
import { PlotsState } from '../../store'

export const TemplatePlotsWrapper: React.FC = () => {
const { nbItemsPerRow, isCollapsed, height, hasItems } = useSelector(
const { nbItemsPerRow, isCollapsed, height, hasItems, errors } = useSelector(
Copy link

Choose a reason for hiding this comment

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

Similar blocks of code found in 6 locations. Consider refactoring.

</th>
</tr>
{revs.map(({ rev, msg }) => (
<tr key={rev}>
Copy link

Choose a reason for hiding this comment

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

Similar blocks of code found in 2 locations. Consider refactoring.

Copy link

codeclimate bot commented Jan 2, 2024

Code Climate has analyzed commit 547cafd and detected 5 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 5

The test coverage on the diff in this pull request is 78.9% (85% is the threshold).

This pull request will bring the total coverage in the repository to 95.1% (-0.1% change).

View more on Code Climate.

@julieg18
Copy link
Contributor Author

julieg18 commented Jan 3, 2024

  • Fix bug where errors don't show unless dvc.yaml changes in current session

Figured out the issue. We actually don't know what type a plot is unless it's built at least once in the current session. If the user starts a new session or reloads the window with a broken plot, the extension will not know the broken plot type and we can't show the error in the Data Series section:

Screen.Recording.2024-01-02.at.6.07.36.PM.mov

One option to fix this is to show the error messaging above the sections below the ribbon:

image

We also could just leave it as is but it seems buggy 🤔 Any other ideas? cc @iterative/vs-code, @shcheklein

@julieg18
Copy link
Contributor Author

julieg18 commented Jan 3, 2024

One option to fix this is to show the error messaging above the sections below the ribbon:
We also could just leave it as is but it seems buggy 🤔 Any other ideas? cc @iterative/vs-code, @shcheklein

Going to open a new pr that adds the error message above the sections and close this one for now. We could at least have a generic error message above all sections as a first iteration and come back to this later if we find a solution for the bug.

@julieg18 julieg18 closed this Jan 3, 2024
@mattseddon mattseddon deleted the add-section-error-message branch March 12, 2024 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product PR that affects product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant