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

Updating Export Button Functionality #591

Conversation

FutzMonitor
Copy link
Collaborator

@FutzMonitor FutzMonitor commented Dec 12, 2022

Pull Request Description

Closes #589 and Closes #571 . This PR aims to update some functionality of the export button that were left out when the CustomTableComponent was being reworked. This includes updating its behavior to be consistent with the behavior of other buttons that are part of the table. Additionally, this PR addresses a bug where the export button leaves behind some - in the comments field.

This PR is not ready to be merged into upstream/main
Below are some things that need to be completed before this PR is ready for review:

  • Have a larger conversation about how the checked rows should behave after a button action (consistency issue)
  • Unit test needs to be guarantee test coverage of updated export functionality

Licensing Certification

FarmData2 is a Free Cultural Work and all accepted contributions are licensed as described in the LICENSE.md file. This requires that the contributor holds the rights to do so. By submitting this pull request I certify that I satisfy the terms of the Developer Certificate of Origin for its contents.

1. Updated the exportCSV() function to only print rows whose IDs can be found in the indexesToAction array.
2. Changes one of the cleanHTML lines to replace new lines with just blank spaces instead of dashes.
3. Made sure the button is disabled when nothing is selected just like the delete button and custom buttons.
@FutzMonitor
Copy link
Collaborator Author

@braughtg Do you believe all the buttons should reset the indexesToAction array? That is to say that when I delete, export, or commit some custom action that afterwards the rows that were checked are now unchecked like it's preparing itself for the next set of instructions? I admit that the delete button is a bit of a special case, but I think maybe we should reset them for all buttons and have them all have that behavior.

@braughtg
Copy link
Member

Do you believe all the buttons should reset the indexesToAction array?

This sounds like a good idea. It is a clear rule on how they should behave and should be intuitable by the user. If it turns out to be inconvenient and we hear feedback otherwise from end users, we can adapt.

So move forward under this assumption.

@FutzMonitor
Copy link
Collaborator Author

FutzMonitor commented Dec 13, 2022

Do you have any idea what may be causing this [unrelated bug] in the Seeding Report? The logs never finish generating and the report tables as the bottom never appear as a result. @braughtg
image

It doesn't do this on my laptop. I tried rebuilding the database and that also doesn't work. It seems to be a bug only I'm running into even though my main is exactly like the upstream/main.

@braughtg
Copy link
Member

braughtg commented Dec 13, 2022 via email

@FutzMonitor
Copy link
Collaborator Author

Maybe a browser specific issue?

Seems to be, it works just fine on Chromium based browsers (tested on Opera GX, Chrome, and Edge).

@FutzMonitor
Copy link
Collaborator Author

FutzMonitor commented Dec 13, 2022

Could you pull this PR and recreate these steps on your machine? @braughtg

  1. Open up any Chromium based browser (Chrome, Opera)
  2. Visit the Seeding Report under the Barnkit tab.
  3. Simply generate the report with the preset dates
  4. Select All the rows and export a CSV.
  5. Open the CSV and observe that it stops at cabbage on the 26th of June instead of the final row which is a beet.
  6. Repeat steps 1-4 but now within FireFox and observe that Firefox doesn't exhibit the same behavior.

I'd like to know if my machine is being buggy or if the export functionality isn't being consistent across browsers. If the latter is the case, this is really concerning because Chromium browsers do not play nice with Vue dev tools which makes it challenging to debug Vue code.

@braughtg
Copy link
Member

Maybe a browser specific issue?

Seems to be, it works just fine on Chromium based browsers (tested on Opera GX, Chrome, and Edge).

This worked fine for me in Chrome and in Firefox on MacOS, and in Firefox in the Dev Container.

@braughtg
Copy link
Member

braughtg commented Dec 15, 2022

I'd like to know if my machine is being buggy or if the export functionality isn't being consistent across browsers.

I saw the same behavior in both Chrome and Firefox on MacOS. The CSV terminated with the 2020-06-26 row for Cabbage when used from both browsers. So neither browser produced the full report.

In Firefox, in the Dev Container I'm getting the abbreviated columns (i.e. mobile view?) where there is no row select check box and thus no way to select all of the rows or to delete or edit rows. This happened no matter what the size of the window was, so this may be a separate issue. But, when I clicked the export button, it exported the whole table. But with fewer columns per row, becuase that is what was being displayed. (This is now its own issue #592 )

So, the issue with the export being cut off seems like it might be due to be a limit on the size of the data that the CSV library you are using. Is that possible?

@FutzMonitor
Copy link
Collaborator Author

FutzMonitor commented Dec 16, 2022

So, the issue with the export being cut off seems like it might be due to be a limit on the size of the data that the CSV library you are using. Is that possible?

It seems like this might be it. I uninstalled and reinstalled FireFox on my machine and that seemed to fix the problem I was having before. Now FireFox is behaving in-line with the Chromium based browsers where the export only exports up to a certain number of rows.

In regards to the mobile view appearing in the dev container, I did not experience similar issues. I got full columns with nothing excluded and had the checkboxes present as well. That should be something we keep an eye out for in case it's not machine dependent and is an independent bug.

image
dev container is presenting normally on my end.

1. Changed the way the csv file object was created. Apparently using the URI encoder limits the amound of data that can be downloaded into the csv. Using a blob object seems to solve this issue.
2. Made it so that if the delete or custom buttons are not present, the select a row feature still appears if the dev allows for csv buttons. This did not exist before, probably an oversight due to export functionality being considered after the fact.
CustomTable Unit Test Changes
1. Updated the unit test to verify that the export button is unclickable when no rows are selected. Made sure downloading one row is assured. Made sure downloading multiple rows is assured.
@FutzMonitor FutzMonitor marked this pull request as ready for review December 16, 2022 02:01
@braughtg braughtg merged commit a3d9228 into DickinsonCollege:main Dec 19, 2022
@FutzMonitor FutzMonitor deleted the updateExportFunctionality branch December 19, 2022 14:54
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.

Table Export includes - characters at end of Comments field. Export Button: Update Functionality.
2 participants