Skip to content

Conversation

@mattseddon
Copy link
Contributor

@mattseddon mattseddon commented Oct 4, 2022

1/2 main <- this <- #2531

Closes #2516

The problem was that the revision field was not being updated by us before trying to render the plot as expected. In order to do that we have to combine the rev field with the filename in the underlying data (no change to the template for these ones).

Successfully rendering the plot then causes a separate issue which is addressed in #2531.

Screenshot:

image

Test project: https://github.com/dberenbaum/example-get-started/tree/flexible_plots (output copied as a new test fixture).

Note: +337,296 of the diff comes from extension/src/test/fixtures/plotsDiff/multiSource.ts

@mattseddon mattseddon added the bug Something isn't working label Oct 4, 2022
@mattseddon mattseddon self-assigned this Oct 4, 2022

if (isMultiView) {
fields.unshift('rev')
return JSON.parse(

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.


const data = getMultiSourceOutput()

export default data
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[F] This uses the regular pattern if we want to include the multi-source plots in a new story then I should be able to wire them up pretty quickly.

Copy link

@qlty-cloud-legacy qlty-cloud-legacy bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 337569 lines exceeds the maximum allowed for the inline comments feature.

@mattseddon mattseddon marked this pull request as ready for review October 5, 2022 03:27
@mattseddon mattseddon marked this pull request as draft October 5, 2022 03:32
Copy link

@qlty-cloud-legacy qlty-cloud-legacy bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 337565 lines exceeds the maximum allowed for the inline comments feature.

@mattseddon mattseddon marked this pull request as ready for review October 5, 2022 03:36
@mattseddon mattseddon marked this pull request as draft October 5, 2022 05:30
Copy link

@qlty-cloud-legacy qlty-cloud-legacy bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 337565 lines exceeds the maximum allowed for the inline comments feature.

@mattseddon mattseddon marked this pull request as ready for review October 5, 2022 07:17
Copy link
Contributor

@julieg18 julieg18 left a comment

Choose a reason for hiding this comment

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

Great work!

@mattseddon mattseddon enabled auto-merge (squash) October 5, 2022 17:27
Copy link

@qlty-cloud-legacy qlty-cloud-legacy bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 337565 lines exceeds the maximum allowed for the inline comments feature.

@qlty-cloud-legacy
Copy link

Code Climate has analyzed commit 1e26fa1 and detected 0 issues on this pull request.

Too many changed lines in diff

View more on Code Climate.

@mattseddon mattseddon merged commit 6c5e2aa into main Oct 5, 2022
@mattseddon mattseddon deleted the fix-flexible-confusion branch October 5, 2022 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Showing wrong info for non-linear flexible plots

3 participants