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

Use native file browsers on GTK-based linux environments #35482

Merged
merged 10 commits into from
May 4, 2023

Conversation

thomashampson
Copy link
Contributor

@thomashampson thomashampson commented Apr 26, 2023

Description of work
Install an extra pacakge from conda-forge called qt-gtk-platformtheme. This package provides the GTK+ 3 platform theme, which makes Qt use native icon themes, fonts, and dialogs on GTK-based environments.

Purpose of work
Since moving to Conda, the file browsers on IDAaaS have become unusable when navigating to large directories. This is because the native browsers weren't being used. Installing this extra package solves the issue.

Further detail of work
Had to change the occt pinning in the conda recipes in order to resolve the dependency chain. It is now consistent with what was already in the developer environments (which is what is used to run all of our tests!).

To test:
(On linux only) Create a new conda environment, activate it, and install mantidworkbench:

conda create -n mantid-gtk-test
conca activate mantid-gtk-test
mamba install -c "thomashampson/label/gtk-test" mantidworkbench

Then launch with workbench and open up a file browser and navigate to a big directory. E.g. this one was causing problems on IDAaaS:
/archive/NDXLARMOR/Instrument/data/cycle_22_3/
The GUI should now remain responsive while the file browser is populating.

I have tested myself on IDAaaS by creating a new conda env and it is working well.

Optionally you can update your development conda environment after checking out this branch with
mamba env update -f mantid-developer-linux.yaml --prune
which will install the new package. Then when you launch your developer build of workbench you should see the gtk theme file browser.

Fixes #34820


Reviewer

Please comment on the points listed below (full description).
Your comments will be used as part of the gatekeeper process, so please comment clearly on what you have checked during your review. If changes are made to the PR during the review process then your final comment will be the most important for gatekeepers. In this comment you should make it clear why any earlier review is still valid, or confirm that all requested changes have been addressed.

Code Review
  • Is the code of an acceptable quality?
  • Does the code conform to the coding standards?
  • Are the unit tests small and test the class in isolation?
  • If there is GUI work does it follow the GUI standards?
  • If there are changes in the release notes then do they describe the changes appropriately?
  • Are the release notes saved in a separate file, using Issue or PR number for file name and in the correct location?
Functional Tests
  • Do changes function as described? Add comments below that describe the tests performed?
  • Do the changes handle unexpected situations, e.g. bad input?
  • Has the relevant (user and developer) documentation been added/updated?

Does everything look good? Mark the review as Approve. A member of @mantidproject/gatekeepers will take care of it.

Gatekeeper

If you need to request changes to a PR then please add a comment and set the review status to "Request changes". This will stop the PR from showing up in the list for other gatekeepers.

jhaigh0 and others added 9 commits April 13, 2023 11:21
Co-authored-by: thomashampson <[email protected]>
This was preventing qt upgrade to 5.15.8, and is now consistent with the
lack of pinning in the conda build recipe
This is holding back tbb in the conda-build, which in turn
prevents qt-gtk-platformtheme from being installed.
It was pinned to =7.5 in recipes and >=7.4 in developer envs.
Build and test jobs have been fine with v7.7.1 so it's safe to move.
@thomashampson thomashampson changed the base branch from main to 0_pin_python_in_build_deps April 26, 2023 10:37
@thomashampson thomashampson changed the base branch from 0_pin_python_in_build_deps to main April 26, 2023 10:37
@thomashampson thomashampson added ISIS Team: Core Issue and pull requests managed by the Core subteam at ISIS Bug Issues and pull requests that are regressions or would be considered a bug by users (e.g. crashing) GUI Issues and pull requests specific to the Mantid Workbench GUI. Reported By User Issues that were found or highlighted by a user/scientist High Priority An issue or pull request that if not addressed is severe enough to postponse a release. Dependencies Pull requests that update a dependency file Linux Only Only on Linux labels Apr 26, 2023
@thomashampson thomashampson marked this pull request as ready for review April 26, 2023 11:18
@thomashampson thomashampson changed the title Native file browsers Use native file browsers on GTK-based linux environments Apr 26, 2023
@robertapplin robertapplin self-assigned this May 4, 2023
Copy link
Contributor

@robertapplin robertapplin left a comment

Choose a reason for hiding this comment

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

Works well for me, good stuff! Just wondered if it is worth adding a comment next to the qt-gtk-platformtheme package in the linux yml file?

Copy link
Contributor

@robertapplin robertapplin left a comment

Choose a reason for hiding this comment

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

Looks good to go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues and pull requests that are regressions or would be considered a bug by users (e.g. crashing) Dependencies Pull requests that update a dependency file GUI Issues and pull requests specific to the Mantid Workbench GUI. High Priority An issue or pull request that if not addressed is severe enough to postponse a release. ISIS Team: Core Issue and pull requests managed by the Core subteam at ISIS Linux Only Only on Linux Reported By User Issues that were found or highlighted by a user/scientist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IDAaaS Freeze and crash when Browsing to Directory with lots of files
4 participants