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

Rename Resolution to Mag (in UI and code where possible) #7378

Closed
philippotto opened this issue Oct 11, 2023 · 10 comments
Closed

Rename Resolution to Mag (in UI and code where possible) #7378

philippotto opened this issue Oct 11, 2023 · 10 comments

Comments

@philippotto
Copy link
Member

philippotto commented Oct 11, 2023

Detailed Description

In code and UI, we should switch to the term Mag everywhere for consistency reason. Quoting @fm3, there should be strategic tooltips which explain the concept.

Context

https://scm.slack.com/archives/C5AKLAV0B/p1695737597207699

@fm3 fm3 changed the title Rename resolution to mag Rename resolution to Mag (in UI and code where possible) Oct 12, 2023
@fm3 fm3 changed the title Rename resolution to Mag (in UI and code where possible) Rename Resolution to Mag (in UI and code where possible) Oct 12, 2023
@fm3
Copy link
Member

fm3 commented Oct 12, 2023

@normanrz @hotzenklotz this is your chance to veto this if you think the term Mag is too obscure ;)

(Though i think at the terminology meeting that resulted in this document https://docs.webknossos.org/webknossos/terminology.html we also agreed on Mag)

@normanrz
Copy link
Member

I think it is more important to be consistent. So, yes for Mag.

@knollengewaechs
Copy link
Contributor

I feel like more strategic tooltips might be tricky in some places of the UI.

  • Annotation view: within the info tab on the right and in the status bar. Both already have a tooltip, where the rendered magnification per layer (and the available mags) are shown. Another tooltip seems too much to me, e.g. if its hovering above an info icon. Thus for these places I propose adding 1-2 sentences below that explain the concept. Linking the docs doesnt work there either, because once the pointer is moved, the tooltip closes.
  • MagSlider to restrict mags (for annotation or volume layer): There the concept is already explained in the tooltip

@fm3
Copy link
Member

fm3 commented Oct 5, 2024

Sounds good!

@knollengewaechs
Copy link
Contributor

Should I also replace the occurences of the term "resolution" in the backend?

@fm3
Copy link
Member

fm3 commented Oct 8, 2024

Backend is a bit tricky, since some of the names of class members are automatically serialized to and from json, which may break backwards compatibility. I can do the backend part (leaving out the spots where compatibility would break) and add it to your PR if that’s ok for you.

@MichaelBuessemeyer
Copy link
Contributor

MichaelBuessemeyer commented Oct 15, 2024

For now the first part in the frontend is done in PR #8111. But renaming local variables & the shader code still needs to be adjusted where possible.

Open TODOs

  • Shader files
  • local variables

Therefore, this issue shouldn't be closed with #8111 and re-scheduled.

@knollengewaechs
Copy link
Contributor

But renaming local variables & the shader code still needs to be adjusted where possible.

done via #8168 😃

@fm3
Copy link
Member

fm3 commented Dec 2, 2024

Nice! Anything missing for this issue, then? Is the shader stuff still open? Otherwise, feel free to close this :)

@knollengewaechs
Copy link
Contributor

no, vars in the shader code were also renamed 😊

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

No branches or pull requests

5 participants