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

Updates to the JS Report #96

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

Updates to the JS Report #96

wants to merge 5 commits into from

Conversation

seebees
Copy link
Contributor

@seebees seebees commented Nov 18, 2022

Enlarging the DataGrid to use the entire browser.
Changing the section and text column to use all available width.
Fix filtering by section
Add a requirements view that lists all requirements across all specifications.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Make the report use the entier browser.
Also change the text and section columns
to autoresize
The value getter for section
should return the string,
not the whole row.
It can be useful to view all requriements
across every specification.
@seebees seebees requested a review from a team as a code owner November 18, 2022 22:01
www/src/App.js Outdated
Comment on lines 145 to 157
const requirements = specifications.flatMap((spec) => spec.requirements);
const total = {
id: "AllSpecifications",
stats: specifications.reduce((total, {stats}) => {
Object.keys(stats).forEach((statName) => {
const stat = total[statName] || new StatsClass()
total[statName] = stat;
stat.onStat(stats[statName]);
});
return total;
}, {}),
requirements,
}
Copy link
Member

Choose a reason for hiding this comment

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

Can you move this to result.js so that we're not recomputing stats every time you load this page? Or at least cache the value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, it is not a lot of work and everything is local,
so I was not too worried, but :)

www/src/App.js Outdated
Comment on lines 158 to 159
specifications
.map((spec) => spec.requirements)
Copy link
Member

Choose a reason for hiding this comment

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

This isn't doing anything

Copy link
Contributor Author

Choose a reason for hiding this comment

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

woops

@@ -0,0 +1,46 @@

Copy link
Member

Choose a reason for hiding this comment

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

nit: just call this file stats.js

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

www/src/App.js Outdated
import specifications from "./result";
import { Stats as StatsClass } from "./stats_class";
Copy link
Member

Choose a reason for hiding this comment

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

nit: Why does the name need Class at the end?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Line 7 import Stats from "./spec"

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