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

volumediscountolist template has linked variables #175

Open
dselman opened this issue Aug 26, 2020 · 17 comments
Open

volumediscountolist template has linked variables #175

dselman opened this issue Aug 26, 2020 · 17 comments

Comments

@dselman
Copy link
Contributor

dselman commented Aug 26, 2020

When editing the volumediscountolist template with the web-components the variables for all rows are linked and get updated simultaneously.

Expected Behavior

Should be able to edit each row independently.

Current Behavior

Variables in all rows are linked.

Possible Solution

We need to distinguish between variables and variables that are inside a list.

Steps to Reproduce

  1. Open the web-components Storybook
  2. Add the volumediscountolist template using the Knobs
  3. Edit one of the rows
  4. All rows are updated (NOK)
@dselman
Copy link
Contributor Author

dselman commented Feb 10, 2021

Looking at this now.

@d-e-v-esh
Copy link
Contributor

@dselman Can I work on this issue?

@dselman dselman removed their assignment Mar 11, 2021
@dselman
Copy link
Contributor Author

dselman commented Mar 11, 2021

Of course.

This is a challenging one I think, based on the preliminary analysis I did. I think the fix needs to go into the CiceroMark <-> Slate transformation in the markdown-transform repository.

Basically, if we hit one of the Variable types and are inside a ListBlock then we should modify the name of the Variable to include the index from the ListBlock. I.e, we will end up with something like:

<ul>
<li>VAR-1</li>
<li>VAR-2</li>
<li>VAR-3</li>
</ul>

I recommend you experiment first with running the Markus command line from markdown-transform and get a good understanding of how the various transformations get applied, and the output from each.

@d-e-v-esh
Copy link
Contributor

Great, I'm looking into this. 👍

@d-e-v-esh
Copy link
Contributor

@dselman I don't know what I am doing wrong but most of the things I try to run in the markus-cli does notactually work.

Can you tell me what I am doing wrong?

C:\Users\Devesh>markus --version
0.12.12

C:\Users\Devesh>markus parse --sample C:\Users\Devesh\Documents\transform.txt   <====
markus <cmd> [args]

Commands:
  markus transform  transform between two formats

Options:
  --version      Show version number                                   [boolean]
  --verbose, -v                                                 [default: false]
  --help         Show help                                             [boolean]

Unknown arguments: sample, parse

C:\Users\Devesh>markus parse                <====
markus <cmd> [args]

Commands:
  markus transform  transform between two formats

Options:
  --version      Show version number                                   [boolean]
  --verbose, -v                                                 [default: false]
  --help         Show help                                             [boolean]

Unknown argument: parse


C:\Users\Devesh> markus draft      <====
markus <cmd> [args]

Commands:
  markus transform  transform between two formats

Options:
  --version      Show version number                                   [boolean]
  --verbose, -v                                                 [default: false]
  --help         Show help                                             [boolean]

Unknown argument: draft

C:\Users\Devesh>

It shows all the commands in the readme here as Unknown argument. I don't think they should return Unknown argument.

@jolanglinais
Copy link
Member

@d-e-v-esh where are you seeing docs for markus parse or markus draft? There should only be markus transform:

17.03.2021@11:33[~]: markus --version
0.12.12
17.03.2021@11:33[~]: markus --help
markus <cmd> [args]

Commands:
  markus transform  transform between two formats

Options:
  --version      Show version number                                   [boolean]
  --verbose, -v                                                 [default: false]
  --help         Show help                                             [boolean]
17.03.2021@11:33[~]:

See the documentation here. parse and draft may be from older versions of the CLI?

@d-e-v-esh
Copy link
Contributor

@irmerk Ya probably. I saw it in this repo

@d-e-v-esh
Copy link
Contributor

@irmerk I think I almost understand how this works. Do you have any idea how I can work on this as markdown-transform is in a different repository and I can't find this in the node modules folder?

@jolanglinais
Copy link
Member

@irmerk Ya probably. I saw it in this repo

Oh yikes! Yes let's open an Issue about this. Maybe the easiest fix would be to have the README just point to the docs I linked.

@irmerk I think I almost understand how this works. Do you have any idea how I can work on this as markdown-transform is in a different repository and I can't find this in the node modules folder?

Hmm, which node_moduels are you looking in? Try web-components/node_modules/@accordproject rather than web-components/packages/ui-contract-editor/node_modules

@d-e-v-esh
Copy link
Contributor

d-e-v-esh commented Mar 18, 2021

@irmerk I created the issue for this. You can assign that issue to me. And I also found the markdown-transform folder in your said path. 👍

To process any changes made do I have to re-run the entire thing with npm run storybook or is there a better way?

@jolanglinais
Copy link
Member

I'm honestly not sure at the moment if there is a better way when editing something within node_modules.

@jeromesimeon
Copy link
Member

jeromesimeon commented Mar 18, 2021

@irmerk I created the issue for this. You can assign that issue to me. And I also found the markdown-transform folder in your said path. 👍

To process any changes made do I have to re-run the entire thing with npm run storybook or is there a better way?

From memory (so apologies if I'm mistaken)

  1. keep storybook running
  2. rebuild which ever package has changed (e.g., in ./packages/ui-contract-editor run npm run build)

I believe storybook will do a hot refresh then (it's a very long build so much better during development)

(but maybe that's a separate question from dependency changes in node_modules ?)

@d-e-v-esh
Copy link
Contributor

Ya seems like that does not work when you make changes in the node modules. The only way I was able to make this work was by npm run storybook which is a bit time-consuming.

@d-e-v-esh
Copy link
Contributor

@dselman It works fine now. The variables look like this. Is this good?

chrome_auLJtPLeA9

Working demo:

CFm5m995Yp

@d-e-v-esh
Copy link
Contributor

@dselman @irmerk Any updates?

@dselman
Copy link
Contributor Author

dselman commented Mar 23, 2021

@dselman @irmerk Any updates?

I think it depends on the implementation. Is there a PR I can review?

@d-e-v-esh
Copy link
Contributor

d-e-v-esh commented Mar 23, 2021

@dselman I just created a PR here. Tell me if this looks good.

I tried many different things but I couldn't find a better way to do this. The main challenge was to create a check to know if the variable was inside a list. So I did the brute-forcing solution inside the list-block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants