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

Rename "Diagnostic Table" to "Distribution Table" #626

Merged
merged 2 commits into from
Sep 18, 2017

Conversation

GoFroggyRun
Copy link
Contributor

Per request in #620, one of the buttons in "build table" option is mislabeled. This PR renames "diagnostic" as "distribution" for this particular button, as well as for the tooltips in the code base.

After this PR, the build table panel would look like the following:

screen shot 2017-08-17 at 3 43 11 pm

@brittainhard could you please review?

cc @martinholmer @MattHJensen

@martinholmer
Copy link
Contributor

@GoFroggyRun, Thanks for the changes in #626.

I'll leave it to others to review the details, but I am curious about what the difference is between static/js/taxbrain-tablebuilder.js and staticfiles/js/taxbrain-tablebuilder.js. Why did you need to change Diagnostic to Distribution in both of those files? What's the difference between the file in the static directory tree and the file in the staticfiles directory tree?

@MattHJensen @brittainhard

@martinholmer
Copy link
Contributor

@GoFroggyRun,
One more question about pull request #626. If we're changing the name on the button, shouldn't we also be changing the name of the CSV (and I guess, Excel) files that are downloaded? Right now those filenames include the phrase diagnostic. All you need to do is change diagnostic to distribution in the filenames.

@GoFroggyRun
Copy link
Contributor Author

GoFroggyRun commented Aug 20, 2017

@martinholmer asked:

I'll leave it to others to review the details, but I am curious about what the difference is between static/js/taxbrain-tablebuilder.js and staticfiles/js/taxbrain-tablebuilder.js. Why did you need to change Diagnostic to Distribution in both of those files? What's the difference between the file in the static directory tree and the file in the staticfiles directory tree?

I might not be the best person to answer this as I am not quite familiar with the infrastructure of webbapp. All I know is that, when database mode were changed, command python manage.py collectstatic needs to be executed so that changes we made in static/js/... will be also copied to staticfiles/js/.... @brittainhard should have much more output than I do.

@martinholmer also said:

One more question about pull request #626. If we're changing the name on the button, shouldn't we also be changing the name of the CSV (and I guess, Excel) files that are downloaded? Right now those filenames include the phrase diagnostic. All you need to do is change diagnostic to distribution in the filenames.

Thanks for noticing this, I have included the name change for those downloaded file in my most recent commit 9a0d292
.

@brittainhard brittainhard merged commit f1f477e into ospc-org:master Sep 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants