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

New feature: Copy a part of the asn1 structure to the clipboard #82

Closed
wants to merge 10 commits into from

Conversation

olibu
Copy link
Contributor

@olibu olibu commented Jan 31, 2024

This is a pull request to add the functionality to copy a part of the asn1 structure to the clipboard.

The following issues are related to this PR:

Adding a further option to download the selection as a file would be easily possible based on this solution. However, it is not part of the current pull request.

@lapo-luchini
Copy link
Owner

I like this! It would probably have sense on the "tree" on the left too.
Right now single-click is bind to "hide tree" but it happens more often by mistake than by on purpose I guess, so a contextual menu like that would be nice there too (maybe with the first element being "collapse/expand" node?)

@lapo-luchini
Copy link
Owner

I imported your commits (almost identically, some whitespace might be slightly different) in branch github-82.
(I use monotone upstream and export to Git, so I can't directly merge your PR as is)

@olibu
Copy link
Contributor Author

olibu commented Mar 31, 2024

I like this! It would probably have sense on the "tree" on the left too. Right now single-click is bind to "hide tree" but it happens more often by mistake than by on purpose I guess, so a contextual menu like that would be nice there too (maybe with the first element being "collapse/expand" node?)

Not sure how to add a context menu to the tree. The tooltip is already taking the space where a context menu might show up. Shall I try to add the context menu from the hex view to the tree instead of collapsing the tree?

@lapo-luchini
Copy link
Owner

lapo-luchini commented Mar 31, 2024

The tooltip is already taking the space where a context menu might show up.

Mhh, right. What if the click "fixed" the popup as a context menu, thus replacing it but with same functionality?
(any suggestion welcome, I'm not really a UI guy myself)

@lapo-luchini
Copy link
Owner

Also, a small bug in current contextual menu: it disappears only when entering and exiting it, but is is possible to "just go away" from it and leaving it on screen until you get back and hover it. (this is small and I'm not even sure it's best to fix it or leave it as is)

@lapo-luchini
Copy link
Owner

I'm also thinking about merging the "esm" branch into "github-82" branch, as the website is fully using modules already anyways (trunk is still not using modules because that complicates usage as a library).

@lapo-luchini
Copy link
Owner

@olibu I changed the contextual menu to just hide string dump when not available, what do you think about it?

@olibu
Copy link
Contributor Author

olibu commented Mar 31, 2024

That's better than the alert. I would change the order of the menu entries. So only the last menu entry is optional.

@olibu
Copy link
Contributor Author

olibu commented Mar 31, 2024

I have a branch in my fork to try a menu at the tree view.
https://github.com/olibu/asn1js/blob/feature/treemenu/dom.js

Is there a way to convert the asn1 to a text shown in the tree view?

@lapo-luchini
Copy link
Owner

Default content is created in the if at line 94 (span.preview) and is created statically for a faster hover…
I wonder if it would be better to do it on-demand on menu creation, or by copying it with something like: node.getElementByClass('preview')[0].innerHTML

@lapo-luchini
Copy link
Owner

OK, I merged current state, but let's keep this open for improvements regarding the context menu, if you want.

@olibu
Copy link
Contributor Author

olibu commented Mar 31, 2024

I updated the context menu in my feature branch https://github.com/olibu/asn1js/tree/feature/treemenu
Any suggestions?

The current context menu on your web site seems to be broken as it shows on top of the screen.

@lapo-luchini
Copy link
Owner

updated the context menu in my feature branch

Thanks! I couldn't import them directly because they didn't include my refactorings, but I tried importing their logic in e41f826.

seems to be broken as it shows on top of the screen.

Strange. It works for me (on both Firefox and Chrome).

@olibu
Copy link
Contributor Author

olibu commented Apr 19, 2024

@lapo-luchini : I've uploaded the feature/treeview branch to my homepage. You can find it at https://olibu.github.io/asn1js/
Waiting for your feedback

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

Successfully merging this pull request may close these issues.

None yet

2 participants