leoheck / kiri

Kiri is a visual tool designed for reviewing schematics and layouts of KiCad projects that are version-controlled with Git.
MIT License
493 stars 35 forks source link

Toggle between new and old view #96

Closed bobatsar closed 1 year ago

bobatsar commented 1 year ago

When viewing the diffs it would be helpful to switch fast between the old and new layout.

Sometimes the diff view is a bit hard to read to see what actually had changed.

It would be good to toggle between new/old with a key press, e.g. t and have another key press to get back to the diff view e.g. d

leoheck commented 1 year ago

Is this a single issue or multiple at once? If it is multiple, could you break into multiple issue reports? It would be really hard for me to understand and work on multiple issues together.

When viewing the diffs it would be helpful to switch fast between the old and new layout.

What do you mean by switch fast? I do understand that sometimes the cached images do not refresh fast enough. Is this what you are talking about? If yes, I am not sure how to improve it. Do you know something that can help me here?

Sometimes the diff view is a bit hard to read to see what actually had changed.

Agree, I would like to improve the colors since colors can help a bit here, but it is a bit hard to understand how the svg filters work.

It would be good to toggle between new/old with a key press, e.g. t and have another key press to get back to the diff view e.g. d

Hmm, what do want to say by toggling between new and old? Are you talking about displaying only one at a time? Then we are not going to have the diff anymore and then it will be even harder to spot differences, isn't it?

bobatsar commented 1 year ago

It is a single feature request.

I would like to toggle between the new and old revision of a sheet/layer (see only one at a time). Usually we see the diff view (with old and new interleaved) but it would be sometimes handy to see only the new or only the old revision. To make it possible to toggle fast, a keyboard shortcut like t would be good. To get back to the normal diff-view another shortcut would be required, e.g. d

leoheck commented 1 year ago

Alright, this may not be hard to implement (maybe) considering my knowledge of Javascript... I will give it a try to see how it would work.

leoheck commented 1 year ago

Why do you think t and d, are good for shortcuts? Considering the mnemonics, it would be good n and o, isnt it? But considering the easy to use.. I would put them on q \ w, maybe

leoheck commented 1 year ago

I have an initial test working now... I just have to decide the shortcuts and improve other things around.. if you want to check if it is usable, here is the branch. https://github.com/leoheck/kiri/tree/toogle-new-old

to make things easier, you can just replace the assets/kiri.js using the one from this branch and you will have this feature to test.

leoheck commented 1 year ago

Ok, now it looks better, I've pushed some changes. Please, check that branch and let me know if it does what you wanted. And let me know if this helps to check the designs, please.

leoheck commented 1 year ago

You can toggle the new commit with q, and toggle the old one with w, you can also toggle the other one when one is hidden.

bobatsar commented 1 year ago

wow, that was fast. yes looks very good. I think this helps to check the designs.

I thought to use only a single key to switch between old and new but your logic with q and w works very good.

Thanks a lot.

bobatsar commented 1 year ago

Would it be possible when viewing old or new to have it also in grey or white? Then when toggling between old and new the color does not distract from the changes.

leoheck commented 1 year ago

I thought the logic of toggle was yours haha, I just combined the use of the shortcuts to avoid having both of the images hidden at the same time since that would be an useless step that could happen.

Regarding the colors... yeah, I was thinking about that, now, I am just not sure how to do that yet since I will have to work with the svg color filters that are kind of hard for me to understand, still.

leoheck commented 1 year ago

I made some progress in the color of the image solo. Please, give it a try and let me know if it is better.. I feel it does not work all the time, but I was toggling images quickly to test.. so, maybe using it as a normal person will work fine... who knows, haha

bobatsar commented 1 year ago

I am now for the next 3 weeks not on a PC, I can check it afterwards.

I also had a short look into the SVG color issues. A simple way would be when only a single image is shown (old or new), to disable the filter in the image temporarily. Then a color could be set. When both are viewed again, the filter has to be added again.

But when diffing old and new the magic filter come into play and I also didn't understand what they are doing :-)

leoheck commented 1 year ago

That would be interesting, but the images are black by default, so I would need to at least map to another whitish color too. Or maybe I could export the images colored, and then show them in the real colors, including layers.. when they are not being changed by the filters. That would be at least something interesting.

Right now the diff color this one

image

The color I could set when we are seeing images individually (almost the same as the unchanged color that I was looking for

image

leoheck commented 1 year ago

I am now for the next 3 weeks not on a PC, I can check it afterwards.

Alright, that's fine.

I will consider this issue finished and close it for now. Please, reopen it or create a new one if you have anything else to discuss. Cheers.

leoheck commented 1 year ago

Hey, this feature was merged in the main already.