DickinsonCollege / FD2School-FarmData2-S23

A fork of FarmData2 that is used for the FarmData2 School Activities.
Other
1 stars 36 forks source link

Seeding Report: Test Cancel Seeding Log Edit #202

Open braughtg opened 1 year ago

braughtg commented 1 year ago

The Seeding Report allows the values within a seeding log (i.e. a row) to be edited by clicking the “Edit” button (i.e. the blue pencil button). When a row is being edited the “Edit” button changes to two buttons. The “Save” button (the green check mark) will save the changes to the database. The “Cancel” button (the brown X) discards any edits and does not change the database.

Notes:

Resources:

Additional Information:

Some additional notes relevant to this issue:

Shahir-47 commented 1 year ago

I would like to work on this!

JinLeeGG commented 1 year ago

I would like to work on this!

won369369 commented 1 year ago

I would like to work on this!

Shahir-47 commented 1 year ago

Hi @braughtg and @johnmaccormick, the seeding report does not show the edit button if the window width or height is less than 900 pixels. So it doesn't show the edit button on my laptop.

image

Here's the code for the custom table: <custom-table data-cy="report-table" :columns="tableColumns" :rows='tableRows' csv-name="seedingReport_" @row-deleted="deleteRowLog" @row-edited='updateRow' @edit-clicked='disableFilters' @edit-canceled='enableFilters' :can-edit='!isMobile' :can-delete='!isMobile'></custom-table>

You can see that the property 'isMobile' needs to be false in order for the prop canEdit to be true. The if condition given below tells that the width and height needs to be greater than or equal to 900 pixels which isn't possible on my device.

if (window.screen.width < 900 || window.screen.height < 900) { this.isMobile = true }

When I change the pixels in the if condition to 500 pixels like shown below:

if (window.screen.width < 500 || window.screen.height < 500) { this.isMobile = true }

The edit button is now clearly visible:

image

For now, I've changed the if condition in the .html file so I can do cypress tests on my laptop but will change it back to how it was previously.

Shahir-47 commented 1 year ago

I found another issue with the seeding report @braughtg @johnmaccormick,

When I press the edit button and select the drop-down menu for the crop, the crop names are repeated twice. For example, the crop name BEET is repeated twice in the drop-down menu as you can see in the screenshots below:

image image

I also used Vue to check the custom table component and found that the second column has a dropdown option with an array that has 222 crop names. I remember from our previous assignment that there were only 111 crop names.

image

In that array you can clearly see down below that the element 0 and element 111 are clearly repeated,

image

image

Because of these duplications, my cypress tests fail.

braughtg commented 1 year ago

the seeding report does not show the edit button if the window width or height is less than 900 pixels. So it doesn't show the edit button on my laptop.

Thanks for pointing this out and adding additional information about the location of the issue and a work around. This seems to be the same issue reported in #230 and in the Upstream FarmData2 at DickinsonCollege/FarmData2#592.

I have seen this issue as well and it has also been reported on screens that render widths > 900 pixels. So, I suspect that your laptop probably does allow rendering of widths greater than 900 pixels, suggesting that this issue is being caused by the way that the browser is reporting widths rather than the 900 number specifically. That said, the workaround you suggest works well for testing at this point.

When you finish the tests, I hope you will consider working on a full solution to #230.

braughtg commented 1 year ago

When I press the edit button and select the drop-down menu for the crop, the crop names are repeated twice.

Good catch! @Shahir-47, would you be willing to create an issue for this bug in the Upstream FarmData2 issue tracker (https://github.com/DickinsonCollege/FarmData2/issues)? That would be super helpful in getting it fixed in the live project.

Once you post the issue, I hope your team will consider working on fixing it!

There was a similar issue with the Areas dropdown in the SeedingInput form (DickinsonCollege/FarmData2#460) which was fixed by PR DickinsonCollege/FarmData2#461. There may be some useful insights there.

Shahir-47 commented 1 year ago

Hi @braughtg, I created a new issue #235 for the duplicate crop names in the dropdown menu.

Shahir-47 commented 1 year ago

@braughtg @johnmaccormick , I fixed the issue #235 and is ready to be reviewed at #237.