Refactoring CustomTableComponent (and other updates) #570

FutzMonitor · 2022-09-17T20:54:51Z

Pull Request Description

Closes #530. This pull request seeks to cover numerous objectives concerning the refactoring of the CustomTableComponent.

Note: This PR isn't ready to be merged yet. The following tasks must be completed prior to code review and merging:

CustomTableComponent.js has to be refactored to no longer use the headers, inputOptions, and visibleColumns props. These changes will ripple and require modifications of the component on top of the additions of checkboxes, regex input, and custom buttons.
Redo the documentation for both the refactored CustomTableComponent and the RegexInputComponent and regenerate the documentation in the project.
The CustomTableComponent.spec.comp.js has to be updated to repair tests that broke due to prop and function changes and new tests have to be written to test the added functionalities.
An example of the refactored CustomTableComponent added to the UI.html file to showcase its new capabilities alongside modifications to the UI.spec.js file to repair testing coverage integrity to the page.
The seedingReport.html file needs to be refactored to make use of the new CustomTableComponent. This means modifying how data is sent to the component and potentially modifying/deleting functions related to props that were deleted in the CustomTableComponent. ~~Alongside this the SeedingReport.spec.js file needs to be modified to repair full page testing coverage.~~
The transplantingReport.html file needs to be refactored to make use of the new CustomTableComponent. This means modifying how data is sent to the component and potentially modifying/deleting functions related to props that were deleted in the CustomTableComponent. ~~Alongside this the transplantingReport.spec.js file needs to be modified to repair full page testing coverage.~~
Create issues for the end-to-end Cypress tests for the Seeding Report and Transplanting report. This isn't restrict solely to myself, if someone would like to contribute to that that would be much appreciated. If not, I will get around to it eventually.

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. These files will be updated with the request changed in the issue 2. This initial commit is to make sure the remote is working correctly

1. Had to add a script path to the example tab because the new test component wasn't loading in otherwise 2. Messing around with making things appear with the new structure of the CustomTableComponent 3. Modifying the template to iterate through the array of objects correctly (in progress) 4. Seriously need to look into updating some of the computed methods that deal with visibility since visibility has been updated

1. Removed number box input type and replaced it with the RegexInputComponent. Its implementation is unfininshed since it still needs to check for valid inputs and make sure that the save button is disabled when an invalid input it inputted. However, these changes will come after the other abstractions are completed. 2. The buttons have been abstracted. I haven't deleted all the code for the predefined buttons since I'm using them as a template but the new general button isn't working as intended. It's not rendering in the table when the conditions for it to be rendered are being met.

1. Added the Boolean type to the CustomTableComponent. This is a checkbox that, when clicked, that entire row has been selected to be passed for some function (whatever that may be). 2. Just like the button, the checkboxes don't want to render into the table. Hopefully these two bugs will be fixed in the next commit

1. The checkboxes and buttons are now rendering. However, whatever is being passed into the table row data (say boolean/button) to indicate that we want a column of checkboxes/unique button is actually being filled into the table data. However, the actual checkbox/button is visible when you want to edit that row. It's as if somehow the states are reversed but we don't want to reverse them back (i.e button/checkbox is rendered but when row is edited they revert to text button/checkbock). We want them to always be rendered as the proper element and not text regardless of whether that row is being edited or not

braughtg · 2022-09-18T17:59:58Z

farmdata2_modules/fd2_tabs/resources/NewCustomTableComponent.js

+                        :data-cy="'td-r'+ri+'c'+ci"
+                        style="text-align:center">
+                            <div v-if="rowToEditIndex!=ri || columns[ci].inputType.type == 'no input'"
+                            :data-cy="'r'+ri+'c'+ci"


This is the part that is rendering the table row if the row is not currently being edited, or if it does not have an input element type. So when the row is not being edited, the check box element is not being rendered (same for the button.). This was fine when all elements were just text when they were not being edited. But now that there are some that are not text (e.g. checkbox, button), you'll need to generalize this or add a special case to handle those elements for when a row is not being edited.

I can get rid of the rowToEditIndex ==ri and that'll render the checkbox/button but the text continues to persist in the box as well. It's also not a good practice because the checkbox/buttons are clickable when you are editing a specific row.

Visual:

update: I can get them to render as intended as long as you leave the row data blank:

I'm not sure I fully understand the issue here. But from what you say, it seems that the buttons and check boxes are not disabled when editing a row unless the row is blank? Is that accurate? That doesn't seem like the behavior we want. We'll want to sort this out if it is still not behaving correctly. Please update on the current status.

This shouldn't be the case anymore. This was when custom buttons were still part of the table row but has been moved to the top of the table. The radio buttons have been updated to be disabled at all times except for when that row is being edited. As far as I remember, rendering isn't an issue anymore.

1. The rendering issue has been partially solved by removing the editing row index inside the Vue conditional. However, for the time being, this means that the element being rendered in (e.g. checkbox/button) are manipulate-able whilest editing rows. This will have to be patched out later (or a better implementation should be explored). 2. Implemented a function for checking all the checkboxes in a column. Currently it changes the Vue data for that row, but it's currently a little buggy and the checkboxes themselves are not checked even though the row itself is checked inside the row data.

1. The rendering issue has been solved, the rows data doesn't populate the div anymore in the table for checkboxes/buttons specifically. If additional functionality is added to the table then those also have to be added to the conditional. 2. Disabled ALL checkboxes/buttons during edits. This choice should be discussed. 3. Select All works but still continues to make no changes to the actual visual tick in the box which is an indication to the user that the box is actually checked 4. Select function declared but currently unoperational

braughtg · 2022-09-29T14:21:53Z

braughtg - Moved this from original description to a comment.

Current Issues
~~Checkboxes/buttons are only rendered in when you edit a table row. Otherwise the HTML text fed into the table's row data is displayed. As shown below.~~

RegexInputComponent is running into major errors. For starters it won't save a number, it'll retain its original value. There's a problem where the regex comparison isn't working properly (e.g. the regex for positive ints only allows '4.5') and when trying to add @input-changed and @match-changed for checking if the value is valid there's an error thrown into the console that says e.apply is not a function.


Positive integer regex allows decimal numbers


Error thrown when I try to incorporate match-changed and input-changed into the html template

fd2_issue.mp4


Video showing that putting a value in doesn't actually change the value

braughtg · 2022-09-29T14:26:44Z

The issue with the input value not changing the value displayed suggests that the row-edited event is not being emitted correctly or that page using the table is not handing the row-edited event properly. I would check to ensure that the event is being emitted, and then if so, ensure that the page using the table is handling it correctly. That event handler in the page will need to change the original row data from which the prop to the table is generated. In the example page, I think that is just an array that is being used. In something like the seeing report, that will be the original data from the api call. The change to that data will be picked up by vue because the rows passed to the table is a computed property. When that computed property changes, the prop will change, when the prop changes, the values displayed in the table should also change.

FutzMonitor · 2022-09-29T15:55:38Z

By observing the original CustomTableComponent I noted that when you change the value inside of the an input box of type number the value being passed becomes a string (inside of the Vue devtools you can see quotation marks surrounding the newly inputted value).

As the author of the RegexInputComponent I am very much aware that strings being inside of the component's input box when that input box's type is number can cause some serious problems:

Image of the value being emitted back

The value emitted back is empty but the box goes back to being white indicating a valid input to the user. Ultimately the value doesn't get save, see video above for example, but I have suspicion that passing a string is causing some critical problems.

1. Removed the selectAll method from this input type since you shouldn't be able to check multiple boxes when only one checkbox can be checked at a time when being edited. This keeps functionality consistent. 2. Used a more clever trick for the select function which just grabs the column index as well to change the checkbox's boolean value which gets rid of the for loop entirely. 3. Removed the header box for the boolean input type, it's just a regular text header now.

1. Custom Buttons now emit an event for the page to handle 2. Added a leftmost column that dictates which rows will be affected by the custom button's event 3. Added a select all feature which allows you to select all the rows 4. The leftmost column is disabled during edit and so are the custom buttons 5. None of these changes apply to the export button. The new thing on the agenda is the RegExp implementation, after that the refactored CustomTable should be done

farmdata2_modules/fd2_tabs/fd2_example/ui.html

farmdata2_modules/fd2_tabs/resources/NewCustomTableComponent.js

1. Disregard changes to the old CustomTableComponent file. I use it as a testing ground for understanding how the table works and thus making the improved version better. 2. The leftmost column needs a bit of work due to some inconsistencies between select all and individual selects of the buttons. 3. The buttons are now using their own emits to grab their things from the child component to the parent page. 4. RegexInputComponent is still not updating and still has the same issues.

1. Fixed a bug where selecting all and then unchecking one of the rows caused the row to effecively 'disappear' and didn't allow for any action on it anymore. 2. Updated the clone method in the UI page to be more secure: uses IDs for rows now and shows a case of updating the IDs for a cloned row because otherwise buttons share the same IDs and checking/unchecking one does the same for the other. 3. Moved the cancel button down to the edit button's space and it only appears when you're editing that one row. It's a bit unpleasing UI wise, but it's not that intrusive.

1. The RegexInComponent finally works with the refactored CustomTableComponent. However, the editing is done in a rather indirect way. Unlike the other types of editing types inside the table, the RegexInputComponent has to use it's @input-changed and pass it to another function to make the necessary changes. Additionally, a change had to be made to the RegexInputComponent. The v-model had to be removed as well.

1. The default value prop is now being provided with the editedRowData.data[i] to be consistent with the rest of the input types in the table. 2. Made some changes to the setMatchVal (previous logic was pretty roundabout) 3. RegexInputComponent had it match emit only on change restored and moved the emitting of the value until after that check to give the table time to update its own isMatch value before adding the value into the editedRowData.data[i]. This fixes a bug where the isMatch value wasn't being updated in time and thus invalid values were being passed into the regex-input which broke it even when the value inside was altered to a valid one.

…into updatedCustomTable

@event

1. UI PAGE CHANGES Removed the match value from the Regex input type, was leftover from initial design which was changed. Renamed 'box' for button box colors to 'buttonClass' to be more inline with what it's actually attached to in the component. Additionally the 'header' was renamed to 'hoverTip' to be more self explanatory. 2. CUSTOMTABLECOMPONENT CHANGES Changes the data-cy values for the buttons to be inline with the data-cy values of the pre-existing buttons. Got rid of any instance of 'btn' and just used 'button' instead. Dropped the '-event' from the button emit and it just now emits the name of the event for the parent page to catch. No more '@eventName-event' just '@event' in the parent page now. Couldn't find anything about the payload being copied for the emit so in the meantime the payload is being copies to make sure an empty payload isn't being sent. Got rid of a potentially unobservable bug of multiple select all boxes being overlapped because of an unnecessary v-for when creating the button in the header. Cleaned the columns from any dated code (boolean types used to be separate and checking for cases that weren't necessary anymore) Updated data-cy for all the row data (e.g row0-date). Moved v-if for the table row data to the top for each HTML element Removed select function for boolean types. Unnecessary. UpdateSelectAll function has been rewritten. NOTE: should receive another lookover for soundness. Linger CustomTableComponent computed methods removed. Deep watch commented out in the meantime. 3. REGEXINPUTCOMPONENT Changes Simply added a comment explaining why the match value is initially null

1. CUSTOMTABLECOMPONENT CHANGES Simplified the logic for selectAll function Removed all debugging code (i.e console.logs)

1. DELETED Deleted the old CustomTableComponent Deleted the old CustomTableComponents unit test Deleted a script from the example.info file for the refactored CustomTableComponent 2. RENAMED Renamed the newCustomTableComponent to CustomTableComponent Same as above for the unit test 3. PROBLEMS All sub-tabs using the CustomTableComponent are broken, including the UI page. Unit test for the CustomTableComponent doesn't want to start.

1. UI PAGE Removed any lingering method names that had the 'new' label to differentiate them from the old CustomTable methods. 2. CUSTOMTABLECOMPONENT.js Removed any lingering mentions of the 'NewCustomTableComponent' from the file.

1. CONFIG INFO Moved the RegexInputComponent's initialization higher to fix a bug where it was being initialized after the CustomTable was trying to use 2. CustomTableComponent.js Added the export at the top for the RegexInputComponent

1. Fixed a bug where the leftmost column was still actionable when editing a row 2. Fixed a bug where the delete button was still enabled while editing. This allowed for an edited row to be deleted during editing which kept the table stuck in an editing state 3. CONCERN: The CustomTable needs to be fed a customButton array. Even if its empty, it needs to be feed one or it runs into rendering errors. This should be discussed. CustomTableComponent Cypress Changes 1. All tests have been updated to reflect the refactoring of the CustomTable 2. The CSV context no longer works due to a OS process issue 3. Save button test isn't working properly RegexInputComponent Changes 1. Some small changes. RegexInputComponent Cypress Changes 1. Made some small changes to account for the isMatch being null initially UI Changes 1. Some small UI changes were made to test the CustomTable visually.

1. Removed (hopefully) remaining vestiges of the 'new' variable names from the UI page. The last remaining instances of it had not been removed from the show/hide column button. Changes to UI End-to-End Test 1. Updated CustomTable tests. Changes to CustomTableComponent 1. Updated the finishEditRow() function to correctly update the JSONObject returned to the page. For whatever reason, it didn't actually affect editing in any visible capacity but this should fix any unforeseeable bugs (hopefully). 2. Gave the CustomButtons prop a default value of empty array to avoid problems when devs do not give a CustomButton prop to their table which caused rendering issues due to a null assignment Changes to CustomTable Unit Test 1. Thanks to the fix of the finishEditRow(), the save tests are now working as expected. 2. CSV context suite has been restored but only works within the vnc (fd2dev container).

1. Reconfigured the table to remove the input types, headers, and visible columns which were all combined into the new tableColumns data. 2. There's a bug where the user, crop, and area maps aren't being properly loading in for editing. 3. The table can display the correct headers according to the type of seeding. 4. Delete and edit functionality need to be updated. Changes in Transplanting Report 1. Reconfigured the table to remove input types, headers, and visible columns which were all combined into the new tableColumns data. 2. There's a bug where the user, crop, and area maps aren't being properly loaded in while editing 3. The table needs to have its edit and delete fuctionalities updated. Changes in UI Page 1. Simple changed a small inconsistency in the columns data.

1. Updated the deleting function so that it can support multiple deletes. 2. Removed debugging code and reminants of old table data. Transplanting Report Changes 1. Updated the deleting function so that it can support multiple deletes. CustomTable Changes: 1. Fixed a bug where regex inputs weren't returning anything in the json object so updating the backend wasn't happening.

Transplanting Report Changes 1. Removed debugging code

farmdata2_modules/fd2_tabs/fd2_barn_kit/seedingReport.html

farmdata2_modules/fd2_tabs/fd2_barn_kit/transplantingReport.html

farmdata2_modules/fd2_tabs/fd2_example/ui.html

1. Added a promise check for the deletion of logs. Changes to Transplanting Report 1. Added a promise check for the deletion of logs. Changes to UI Page 1. Added a promise check for the deletion of rows Changes to CustomTableComponent 1. Added a deep watch that's supposed to keep an eye on the columns prop and update it on the table end. It's currently not functioning correctly.

farmdata2_modules/fd2_tabs/resources/CustomTableComponent.js

1. Returning the columns prop in the CustomTable through a computed propety now. The generation of the user, area, and crop name arrays have also been separated into their own properties in anticiaption of another issue. 2. Removed old code CustomTableComponent Changes 1. Removed watcher. 2. Left updatedColumns piece in anticipation that it might be used again

1. Commented out updatedColumns piece

1. Realized that I didn't save the file and thus my removal of old code wasn't present in the last commit. Transplanting Report Changes 1. Made sure it was using the same computed property structure as the Seeding Report page.

FutzMonitor · 2022-09-18T22:59:43Z

farmdata2_modules/fd2_tabs/resources/NewCustomTableComponent.js

+                        :data-cy="'td-r'+ri+'c'+ci"
+                        style="text-align:center">
+                            <div v-if="rowToEditIndex!=ri || columns[ci].inputType.type == 'no input'"
+                            :data-cy="'r'+ri+'c'+ci"


I can get rid of the rowToEditIndex ==ri and that'll render the checkbox/button but the text continues to persist in the box as well. It's also not a good practice because the checkbox/buttons are clickable when you are editing a specific row.

Visual:

update: I can get them to render as intended as long as you leave the row data blank:

farmdata2_modules/fd2_tabs/fd2_example/ui.html

farmdata2_modules/fd2_tabs/resources/RegexInputComponent.js

farmdata2_modules/fd2_tabs/fd2_example/ui.html

farmdata2_modules/fd2_tabs/resources/CustomTableComponent.js

farmdata2_modules/fd2_tabs/fd2_barn_kit/seedingReport.html

braughtg

Looks good! Great work getting this big task together!

FutzMonitor added 5 commits September 6, 2022 17:07

Added Two New Files for the Updated CustomTableComponent

f9d3310

1. These files will be updated with the request changed in the issue 2. This initial commit is to make sure the remote is working correctly

braughtg reviewed Sep 18, 2022

View reviewed changes

FutzMonitor added 2 commits September 21, 2022 15:15

FutzMonitor added 2 commits October 1, 2022 17:20

braughtg reviewed Oct 6, 2022

View reviewed changes

farmdata2_modules/fd2_tabs/fd2_example/ui.html Outdated Show resolved Hide resolved

braughtg reviewed Oct 6, 2022

View reviewed changes

farmdata2_modules/fd2_tabs/resources/NewCustomTableComponent.js Outdated Show resolved Hide resolved

FutzMonitor added 9 commits October 12, 2022 20:13

Merge branch 'main' of https://github.com/DickinsonCollege/FarmData2 …

8afe70f

…into updatedCustomTable

More Code Review Changes

2995420

1. CUSTOMTABLECOMPONENT CHANGES Simplified the logic for selectAll function Removed all debugging code (i.e console.logs)

This was referenced Nov 5, 2022

Seeding Report: Restoring Functionality of End-to-End Tests #577

Open

Transplanting Report: Restoring Functionality of End-to-End Tests #578

Open

FD2 Example : Restoring Functionality of End-to-End Tests #580

Open

FutzMonitor added 3 commits November 9, 2022 15:46

FutzMonitor added 4 commits November 27, 2022 12:53

Merge branch 'main' into updatedCustomTable

3fdfc11

Added Documentation

ceaab15

Transplanting Report Changes 1. Removed debugging code

FutzMonitor marked this pull request as ready for review November 30, 2022 22:43

braughtg reviewed Dec 1, 2022

View reviewed changes

farmdata2_modules/fd2_tabs/fd2_barn_kit/seedingReport.html Show resolved Hide resolved

braughtg reviewed Dec 1, 2022

View reviewed changes

farmdata2_modules/fd2_tabs/fd2_barn_kit/transplantingReport.html Show resolved Hide resolved

braughtg reviewed Dec 1, 2022

View reviewed changes

farmdata2_modules/fd2_tabs/fd2_example/ui.html Show resolved Hide resolved

braughtg reviewed Dec 4, 2022

View reviewed changes

farmdata2_modules/fd2_tabs/resources/CustomTableComponent.js Outdated Show resolved Hide resolved

FutzMonitor added 3 commits December 8, 2022 11:48

CustomTable Changes

9abe9e6

1. Commented out updatedColumns piece

Seeding Report Changes

476f96d

1. Realized that I didn't save the file and thus my removal of old code wasn't present in the last commit. Transplanting Report Changes 1. Made sure it was using the same computed property structure as the Seeding Report page.

FutzMonitor requested a review from braughtg December 12, 2022 18:07

FutzMonitor commented Dec 12, 2022

View reviewed changes

braughtg approved these changes Dec 12, 2022

View reviewed changes

braughtg merged commit 3c6a1b2 into DickinsonCollege:main Dec 12, 2022

FutzMonitor deleted the updatedCustomTable branch December 12, 2022 19:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactoring CustomTableComponent (and other updates) #570

Refactoring CustomTableComponent (and other updates) #570

FutzMonitor commented Sep 17, 2022 •

edited

Loading

braughtg Sep 18, 2022

FutzMonitor Sep 18, 2022

braughtg Dec 12, 2022

FutzMonitor Dec 12, 2022

braughtg commented Sep 29, 2022 •

edited

Loading

braughtg commented Sep 29, 2022

FutzMonitor commented Sep 29, 2022

FutzMonitor Sep 18, 2022

braughtg left a comment

Refactoring CustomTableComponent (and other updates) #570

Refactoring CustomTableComponent (and other updates) #570

Conversation

FutzMonitor commented Sep 17, 2022 • edited Loading

braughtg Sep 18, 2022

Choose a reason for hiding this comment

FutzMonitor Sep 18, 2022

Choose a reason for hiding this comment

braughtg Dec 12, 2022

Choose a reason for hiding this comment

FutzMonitor Dec 12, 2022

Choose a reason for hiding this comment

braughtg commented Sep 29, 2022 • edited Loading

braughtg commented Sep 29, 2022

FutzMonitor commented Sep 29, 2022

FutzMonitor Sep 18, 2022

Choose a reason for hiding this comment

braughtg left a comment

Choose a reason for hiding this comment

FutzMonitor commented Sep 17, 2022 •

edited

Loading

braughtg commented Sep 29, 2022 •

edited

Loading