Media opening (slight reorder & tidy of #1058) #1223
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The return codes for GUI commands, are interpreted as 1 for WSL and 0 for others. This commit wraps the expected return codes into a helper in `platform_code` - `successful_GUI_return_code()`. Tests added.
There is a difference in path types for Windows and Unix systems. Windows uses backward slashes, whereas other systems use forward slashes. Since a media link in WSL will need an absolute path, such normalization is required. This commit place the logic to normalize a file path in a helper in `platform_code`. Tests added.
Tests added and amendments made by Preet Mishra to make it usable for MessageLinkButton and PopUpConfirmationView later. Tests and code further amended by Sarthak Garg to support using report_* methods for UI calls, and refactoring *_media() methods. Co-authored-by: Preet Mishra <[email protected]> Co-authored-by: Sarthak Garg <[email protected]>
This commit adds a method `process_media` to handle media links, i.e., it Downloads and Shows the media in the default OS-specific explorer. Tests added and amended.
Amendment made by Sarthak Garg to use new `report_*` methods for footer notifications. These are to show download status in footer. Co-authored by: Sarthak Garg <[email protected]>
neiljp
added
the
PR ready to be merged
12 tasks
The parameter is useful for prompts that have long descriptions.
Use 'center' location for show_media_confirmation_popup() for better visibility of the popup message. Test amended.
neiljp
force-pushed
the
review-original-1058-5-with-reorder-and-tidy
branch
from
a9d3837
to
11af727
Compare
This was referenced May 16, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This is a slight tidy and re-ordering of #1058, which itself has quite a lot of history via earlier PRs. I've left #1058 as it is right now, other than the changes I made semi-recently to make it cleanly mergeable via a rebase onto then-main. When this is merged then that and a few other PRs can be closed too.
This functions fairly well right now and fulfills a feature, though I can foresee quite a few updates following this, which I'll push out to another issue. For now I'm keen to get this merged.
Tested?
Notes & Questions
I'd like to merge this very soon, but feedback is welcome, as it was on #1058 - the functionality should not have changed from that.