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

Feature: making the Help menu more helpful via weblinks (re-do of #5094) #5399

Merged
merged 9 commits into from Dec 19, 2022

Conversation

psobolewskiPhD
Copy link
Member

Description

This PR is a re-do of #5096 due to the move to app-model.
It populates the Help menu with links to helpful resources, including the Getting started guide, Layers guides, examples, and release notes (for releases only, disable for dev):
image

While having access to some of these resources within napari could be nice, it seems common across programs I use (e.g. VS Code) for the Help menu to just link out to webpages.
I'm kind of tempted to add: Report Issue linking to GitHub issues?

Type of change

  • Bug-fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

References

Closes #5094

How has this been tested?

  • example: the test suite for my feature covers cases x, y, and z
  • example: all tests pass with my change
  • example: I check if my changes works with both PySide and PyQt backends
    as there are small differences between the two Qt bindings.

Final checklist:

  • My PR is the minimum possible work for the desired functionality
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • If I included new strings, I have used trans. to make them localizable.
    For more information see our translations guide.

@codecov
Copy link

codecov bot commented Dec 11, 2022

Codecov Report

Merging #5399 (709230e) into main (9eeab55) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #5399      +/-   ##
==========================================
- Coverage   89.01%   89.01%   -0.01%     
==========================================
  Files         594      596       +2     
  Lines       50237    50264      +27     
==========================================
+ Hits        44720    44744      +24     
- Misses       5517     5520       +3     
Impacted Files Coverage Δ
napari/_app_model/_app.py 100.00% <100.00%> (ø)
napari/_app_model/_tests/test_help_menu_urls.py 100.00% <100.00%> (ø)
napari/_app_model/actions/_help_actions.py 100.00% <100.00%> (ø)
napari/_app_model/constants/_commands.py 98.36% <100.00%> (+0.21%) ⬆️
napari/_qt/_qapp_model/qactions/_help.py 86.66% <100.00%> (ø)
napari/layers/tracks/_track_utils.py 88.06% <0.00%> (-3.41%) ⬇️
napari/layers/tracks/tracks.py 94.29% <0.00%> (-0.39%) ⬇️
napari/utils/theme.py 89.79% <0.00%> (+0.68%) ⬆️
napari/utils/info.py 81.44% <0.00%> (+1.03%) ⬆️
napari/_qt/__init__.py 56.66% <0.00%> (+6.66%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@goanpeca
Copy link
Contributor

This is awesome @psobolewskiPhD, thanks for working on this.

I'm kind of tempted to add: Report Issue linking to GitHub issues?

I would suggest we link to a documentation page where we can have troubleshoot and explain how users can report issues on github (a lot of users might not have a github account)

However, we could add this 'report issue on github' on dev mode only :).

Cheers!

id=CommandId.NAPARI_GETTING_STARTED,
title=CommandId.NAPARI_GETTING_STARTED.title,
callback=lambda: webbrowser.open(
f'https://napari.org/{VERSION}/tutorials/start_index.html'
Copy link
Contributor

@goanpeca goanpeca Dec 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@psobolewskiPhD how about we move all these URLS to a constant dictionary next to VERSION that way we can write a simple test that checks if the urls keep working moving forward.

We can use requests to write this test.

Something like:

HELP_URLS = {
    "getting-started":, f'https://napari.org/{VERSION}/tutorials/start_index.html',
     "tutorials": f'https://napari.org/{VERSION}/tutorials/index.html',
    # ...
}

        callback=lambda: webbrowser.open(HELP_URLS['getting-started'])

And the test could be something like

import requests  # See https://requests.readthedocs.io/en/latest/

from napari._app_model.actions._help_actions import HELP_URLS


def test_help_urls():
    for key, url in HELP_URLS.items():
        r = requests.head(url)
        r.raise_for_status()  # Will raise for 4xx and 5xx codes. See: https://requests.readthedocs.io/en/latest/api/#requests.Response.raise_for_status

(Did not test any of this code... but it should work or almost :-p)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @psobolewskiPhD, left a comment for a small tweak so we can test the urls keep working 🧪

Love it! 🥰

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a ton @goanpeca
Really helpful!

Implemented the test slightly differently:
I wanted the test to return info about each link, because I knew release_notes doesn't work for dev (aka CI).
So I read up on parametrize from pytest and used that to check each url in the dict, skipping the release notes one.

Copy link
Contributor

@goanpeca goanpeca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @psobolewskiPhD, left a comment for a small tweak so we can test the urls keep working 🧪

@github-actions github-actions bot added the tests Something related to our tests label Dec 11, 2022
@psobolewskiPhD
Copy link
Member Author

psobolewskiPhD commented Dec 11, 2022

This is awesome @psobolewskiPhD, thanks for working on this.

I'm kind of tempted to add: Report Issue linking to GitHub issues?

I would suggest we link to a documentation page where we can have troubleshoot and explain how users can report issues on github (a lot of users might not have a github account)

Like this one?
https://napari.org/stable/index.html#help
Could be opened by menu item:
"Get help"

(just noticed it doesn't mention the zulip, maybe should update that section, and maybe it should be a dedicated getting more help page under Usage.)
Issue: napari/docs#66

However, we could add this 'report issue on github' on dev mode only :).

This is definitely doable, already doing a check for that.
Edit: honestly, given that we're alpha, a link to GitHub issues page is probably not terrible in a release version too...

@goanpeca
Copy link
Contributor

Edit: honestly, given that we're alpha, a link to GitHub issues page is probably not terrible in a release version too...

This could result in a lot of people reporting the same issue over and over, since my experience is that a lot of users do not search if an issue has been previously reported before opening a new one 🤔

Copy link
Contributor

@liu-ziyang liu-ziyang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

I don't want to nitpick on the choosing and ordering of what links should go here, because this is apparently better than not having anything. But would be great to eventually refine the list of links that goes in here

@psobolewskiPhD
Copy link
Member Author

LGTM!

I don't want to nitpick on the choosing and ordering of what links should go here, because this is apparently better than not having anything. But would be great to eventually refine the list of links that goes in here

I'm open to nitpicks!

@psobolewskiPhD
Copy link
Member Author

psobolewskiPhD commented Dec 12, 2022

@goanpeca I added a link to GitHub issues for dev versions:
image

Here's a live release (0.4.17):
image

I'm using the conditional when so that the items are present or not, rather than greyed out.
The general order is based on VS Code:
image

Edit: Could add another divider before Release Notes / GitHub

@goanpeca
Copy link
Contributor

@psobolewskiPhD looks great 🚀

Edit: Could add another divider before Release Notes / GitHub
👍🏼 like it

On the dev version text I would suggest, "Report an issue on Github" as it feels more generic, you could also report a feature request, while fixing a bug ;-)

(It is not all bugs 😆)

@psobolewskiPhD
Copy link
Member Author

@goanpeca Here's the new dev version:
image
and release:
image

Copy link
Contributor

@goanpeca goanpeca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it 🚀

Copy link
Member

@melissawm melissawm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me, thanks @psobolewskiPhD !

Copy link
Member

@jni jni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love this, thank you @psobolewskiPhD!

@brisvag
Copy link
Contributor

brisvag commented Dec 19, 2022

We have a bunch of approvals already fopr a while, let's get this in :)

@brisvag brisvag merged commit c9053cd into napari:main Dec 19, 2022
@Nadalyn-CZI
Copy link
Contributor

Peter, how can I install the version of napari that has the links in the help menu?

@psobolewskiPhD
Copy link
Member Author

Peter, how can I install the version of napari that has the links in the help menu?

You would need to install the dev version, either like in the contributor docs:
https://napari.org/stable/developers/contributing.html#setting-up-a-development-installation
or you could try just installing from GitHub directly using pip:
pip install git+https://github.com/napari/napari.git
but for this to work I think you will need an env with pyqt5 or pyside2

@Nadalyn-CZI
Copy link
Contributor

Nadalyn-CZI commented Mar 4, 2023 via email

@Czaki Czaki modified the milestones: 0.5.0, 0.4.18 Jun 23, 2023
@Czaki Czaki mentioned this pull request Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
qt Relates to qt tests Something related to our tests UI/UX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Help menu doesn't actually offer any help
8 participants