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

Double clicking the words in sentence builder #759

Closed
DavidLuterancik opened this issue Oct 5, 2020 · 20 comments
Closed

Double clicking the words in sentence builder #759

DavidLuterancik opened this issue Oct 5, 2020 · 20 comments
Labels
help wanted Extra attention is needed

Comments

@DavidLuterancik
Copy link
Contributor

Is your feature request related to a problem? Please describe.
With translate sentence, only drag&drop is available. To improve user experience I suggest adding double click.

Describe the solution you'd like
Double clicking :

  • adds the word at the end of the sentence
  • removes the word from sentence into the word pool

Additional context
image

@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the label feature_request to this issue, with a confidence of 0.75. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

@kantord
Copy link
Collaborator

kantord commented Oct 5, 2020

Great idea! Maybe it could even just be single clicking?

@kantord
Copy link
Collaborator

kantord commented Oct 5, 2020

@allcontributors please add @DavidLuterancik for ideas

@allcontributors
Copy link
Contributor

@kantord

I've put up a pull request to add @DavidLuterancik! 🎉

@HugoLiconV
Copy link

I could take a look at this. Could someone help me to find the code for this part?

@kantord
Copy link
Collaborator

kantord commented Oct 6, 2020

@HugoLiconV amazing!!! it's this file: https://github.com/kantord/LibreLingo/blob/master/workspaces/web/src/components/ChipsChallenge/index.svelte

let me know if you have any more questions

@kantord kantord added this to In progress in Hacktoberfest 2020 Oct 6, 2020
@HugoLiconV
Copy link

Hi @kantord. I started working with this and I was able to finish the feature, but now I'm having some bugs while changing the position of the chips.
bug video

And the value of the variables I'm using to hold the answers and the options are being updated but the view is not.
bug video

any idea of what could it be?

@kantord
Copy link
Collaborator

kantord commented Oct 8, 2020

Without seeing the code, I'm not sure why the issues might be, but here's one idea.

SortableJS has it's own internal state management, and perhaps the changes that you are making are not propagated to this internal store.

Probably there's some API in SortableJS that you need to call to make this update externally (best case) or you need to re-create sortable every time this happens (worse case).

If you cannot find such an API or an example for this, you can try asking on their GitHub page.

@HugoLiconV
Copy link

I think the bug was created when I removed get.

+{#each $answer as chip, index}
-{#each get(answer) as chip}

The problem now is that if I add get again the on click event doesn't work because it updates the values of the variables but it doesn't update the view.

I created this codesandbox where is easier to check the bug.

This is the first time I use svelte so I don't know what the error could be.

@kantord
Copy link
Collaborator

kantord commented Oct 9, 2020

So If I understand correctly, what you did was probably a good idea:

+{#each $answer as chip, index}
-{#each get(answer) as chip}

But probably the store inside Sortable is outdated, and it's messing up the indices of the elements, right?

Maybe there's a better way, but I was thinking that this could be fixed by re-creating sortable every time when the store is updated from the svelte side.

The problem is, that the recreation doesn't happen, because only the items are dependent on the value:

      <div
        class="chips"
        use:sortable="{{ items: answer, options: { group: 'chips', forceFallback: false } }}">
        {#each $answer as chip, index}
          <span class="chip" on:click="{handleAnswerClick(chip, index)}">
            <spain class="tag is-medium">{chip}</spain>
          </span>
        {/each}
      </div>

@kantord
Copy link
Collaborator

kantord commented Oct 9, 2020

I unfortunately don't have more time time, but I think the problem is that the update feature here:

    return {
      update(items) {
        sortable.destroy();
        sortable = new Sortable(node, options);
      },
      destroy() {
        sortable.destroy();
      }
    };

Doesn't get called when it should be. Perhaps you could try asking at the Svetle community?

@kantord
Copy link
Collaborator

kantord commented Oct 13, 2020

@HugoLiconV how is it going? Can I help?

@HugoLiconV
Copy link

@kantord Yesterday I was working on it but I wasn't able to fix the error, today I'll try to ask for help

@HugoLiconV
Copy link

Hi, @kantord I just want it to give you an update. I've been working on this but I haven't been able to solve it, and I'm out of ideas now. I asked in stack overflow and I created an issue on SortableJS but I haven't received an answer yet.

@kantord kantord added help wanted Extra attention is needed and removed Hacktoberfest feature_request good first issue Good for newcomers labels Oct 21, 2020
@kantord kantord pinned this issue Oct 21, 2020
@kantord kantord removed this from In progress in Hacktoberfest 2020 Oct 21, 2020
@HugoLiconV
Copy link

Well, someone answered on the issue I created on SortableJS. SortableJS/Sortable#1946

@kantord kantord added this to To do in CSS-in-JS Jan 13, 2021
@kantord kantord removed this from To do in CSS-in-JS Jan 13, 2021
@chickendude
Copy link
Contributor

chickendude commented Feb 19, 2021

To be honest i don't think being sortable is a very high priority, i think just clicking to add/remove would be enough (and would be much more convenient than the current implementation). Ideally, you should be thinking of the sentence from start to finish anyway, i.e. not:

dog -> dog big -> the dog big -> the dog is big -> the blue dog is big

but:

the -> the blue -> the blue dog -> the blue dog is -> the blue dog is big

@kantord
Copy link
Collaborator

kantord commented Feb 19, 2021

@chickendude

Even though I don't agree with this part:

Ideally, you should be thinking of the sentence from start to finish anyway

I kinda agree with the overall conclusion. It's way more important for it to be stable and easy to use. There are other bugs with the sentence builder as well.

Would you have some time preparing the PR to remove the dragging functionality and add clicking instead?

@chickendude
Copy link
Contributor

Would you have some time preparing the PR to remove the dragging functionality and add clicking instead?

I'm not much of a js person and have never used Svelte, but i'll definitely take a look! I think it would make sentence-building questions much easier/more convenient. I can see being able to rearrange the words in the final sentence as a nice feature to have, but if it's one over the other i'd prefer clicking over dragging.

@kantord
Copy link
Collaborator

kantord commented Feb 21, 2021

Thanks @chickendude, amazing!

@kantord
Copy link
Collaborator

kantord commented Feb 21, 2021

Fixed by @chickendude here: #1098

@kantord kantord closed this as completed Feb 21, 2021
@kantord kantord unpinned this issue Feb 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants