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

feat: create and edit assignment labels for ranges #2971

Merged
merged 4 commits into from Mar 22, 2024

Conversation

hamed-musallam
Copy link
Member

No description provided.

@hamed-musallam hamed-musallam linked an issue Mar 19, 2024 that may be closed by this pull request
Copy link

cloudflare-pages bot commented Mar 19, 2024

Deploying nmrium with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5c9e736
Status: ✅  Deploy successful!
Preview URL: https://6f370749.nmrium.pages.dev
Branch Preview URL: https://add-assignment-in-range.nmrium.pages.dev

View logs

@hamed-musallam
Copy link
Member Author

@lpatiny

Would you like to add a new property to the range preferences for toggling the visibility of the assignment label column? If so, what would be the default value?

We currently have one property to toggle the visibility of the range and signal assignment columns. I adjust the visibility of the assignment label based on this property as well

Screenshot 2024-03-19 at 11 52 35

@hamed-musallam
Copy link
Member Author

hamed-musallam commented Mar 19, 2024

if you require a new property to toggle the visibility, I have created a PR in nmr-load-save for that: https://github.com/cheminfo/nmr-load-save/pull/367. Otherwise, I will close this PR.

What do you think? Is a new property necessary, or is what we currently have sufficient?

@lpatiny
Copy link
Member

lpatiny commented Mar 19, 2024

I think it should not be visible by default.
I would also prefer it is the second column just after # and before delta.
Is there a way to display the assignment on the spectrum ? This would be very practical. We will have conflict between labels that overlaps but we will solve this later. It could be on the same line as the 12 of this image. This would also require one more toggle in the range header ...
image

@lpatiny
Copy link
Member

lpatiny commented Mar 19, 2024

Please also move this icon

image

just after the sum symbol because there are 'linked'.

@hamed-musallam hamed-musallam changed the title feat: set range assignment label feat: create and edit assignment labels for ranges Mar 22, 2024
@hamed-musallam hamed-musallam merged commit 31297a2 into main Mar 22, 2024
11 checks passed
@hamed-musallam hamed-musallam deleted the add-assignment-in-range branch March 22, 2024 08:15
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.

Assignment in range and signal
2 participants