pik-gane / vodle

We develop an interactive, consensus-oriented group decision app
https://twitter.com/vodle_it
GNU Affero General Public License v3.0
25 stars 16 forks source link

Unexpected behavior on "draft poll" page #256

Closed asumandang closed 11 months ago

asumandang commented 1 year ago

Issue

Evidence

https://github.com/pik-gane/vodle/assets/32838966/8d76eceb-c0eb-40e9-b94a-7bfa15a4e10f

ghost commented 1 year ago
👇 Click on the image for a new way to code review #### [![Review these changes using an interactive CodeSee Map](https://s3.us-east-2.amazonaws.com/maps.codesee.io/images/github/pik-gane/vodle/256/43716224/143fec3db05ba981c8b4474ed9c48297a37a5415.svg)](https://app.codesee.io/r/reviews?pr=256&src=https%3A%2F%2Fgithub.com%2Fpik-gane%2Fvodle) #### Legend CodeSee Map legend
mensch72 commented 1 year ago

Hi @asumandang , I'm now testing this. Regarding the greyed-out "Ready" button, I can still get that unfortunately, e.g. here: image When I then click the time field and exit the popup again, nothing changes. But when I click the time field and change the time, the "Ready" button becomes active. Alternatively, when I change any other field, the "Ready" button also becomes active. Also, when I add a third option, the button becomes active. But: if I then delete the third option, it gets greyed out again. Also, when I'm in the situation of the screenshot above, then focus the "b" field, and then press tab, then the focus moves to the "+" button for a split second and then moves back to the "b" field, so it refuses to leave the "b" field as if that field was invalid. When I then click the "a" field, the focus starts moving very fast between the "a" and "b" fields.

Can you reproduce these effects?

I believe you are right that this is an issue with validation, but it seems it hasn't been solved yet :-(

asumandang commented 1 year ago

Good day @mensch72 ! Unfortunately, I couldn't replicate the "Ready" greying out from my side. Was there something I miss? What I did:

  1. Create a poll from 'My Polls'
  2. Input the values you inputted in your screenshot image

However, I was able to replicate the issue regarding the option tabbing. Pushed a fix regarding this issue. Before (Bug Replication): https://drive.google.com/file/d/16fjtpOvBR4s0Y6TXYvTQenPj7MzftzpN/view?usp=sharing After: https://drive.google.com/file/d/1dl02Vqs-eiqAQ1JSHxemmIba5pQs4kDr/view?usp=sharing

Side note I did notice another issue regarding the option deletion and I was hoping you'd allow me to do it in another PR to avoid bloating this PR with too many different issues.

Issue I noticed that the added options are saved but the deleted options are not saved. E.g. You have options a, b, c. You add option d and deleted option b. On reload, you will have option a, b, c, d.

mensch72 commented 12 months ago

Hi, sorry for being slow again - I was on vacation.

Regarding the additional issue you identified: Thank you for spotting this! And of course I'd be happy if you still offer to fix that in a separate PR.

mensch72 commented 12 months ago

Regarding the tabbing issue: Your second commit indeed seems to have solved that issue.

I'd like to understand your solution... Can you explain what the startWith({}) does?

asumandang commented 11 months ago

Regarding the tabbing issue: Your second commit indeed seems to have solved that issue.

I'd like to understand your solution... Can you explain what the startWith({}) does?

startWith in rxjs allows you to include an initial value that will be emitted before any other values emitted by the observable.

In other words, this observable will startWith the value {}

What happened in the below code...

existingOptionName$(currentControlName: string): Observable<string[]> {
    return this.formGroup.valueChanges.pipe(
      startWith({}),
      map(values => {
        const optionNameKeys = Object.keys(values as Object).filter(k => { return k.includes('option_name') && k !== currentControlName })

RCA: This async validator caused the form to be in a pending state because the values started off as undefined (an error in the validation)

Solution: To solve it, values initially must not be undefined but with the value {} so that validator function does not throw error.