Cannot install when admin is unavailable

TL;DR Issues to be solved
After pair-programming with @agvarghe we saw that the main issue was that powershell.exe was not recognized as a command on his side. Once the C:\Windows\System32\WindowsPowerShell\ path was added to the Path system variable, everything started to work. So, there are a couple of things to be done with this issue:

[ ] Forward errors in a dialog box
[ ] Do not assume C:\Windows\System32\WindowsPowerShell\ is on Path. Instead of invoking powershell.exe, invoke the full path - Windows will always install it in that location.
[ ] Following on @kmahajan-cadfem's comment, it looks like there is some issues on the download process. We might open a new issue to solve this.

Issue description
🔍 Before submitting the issue

[X] I have searched among the existing issues
[X] I am using a Python virtual environment

🐞 Description of the bug
When performing Python installation , i am unable to install python
📝 Steps to reproduce

Launch the Ansys Python Manager application
Select "Install Python" Tab
Under "Python version" select Python3.10
Proceed with Install,
No prompts/or tabs present.

💻 Which operating system are you using?
Windows
📀 Which ANSYS version are you using?
2023R1
🐍 Which Python version are you using?
3.10
📦 Installed packages
-

@RobPasMue, let's follow up on this.

Hi @agvarghe - I tried replicating your steps but I couldn't find the bug... Please have a look at the attached vide showing my steps and replicating your operations. Could you share with me more information about your failure?
https://user-images.githubusercontent.com/37798125/234488819-0bf2681d-85c9-49eb-aa7b-908f61ea960b.mp4

Hi @RobPasMue,
I am also facing same behavior, following is a screenshot, GUI remains in this state unless I kill the process.
I should also note that I dont have admin rights on my windows machine to Install anything and also have python already installed on my machine.
is there any log or is there any switch for the exe with which I can create log?

Hmm interesting. If you do not have admin rights, then it's true that you won't be able to install Python itself. We can implement some safety checks to see if your user has admin rights or not.
However, you should still see your default Python install on the second tab called "Create virtual environments" and thus you can continue with the rest of the experience

If you do not have admin rights, then it's true that you won't be able to install Python itself. We can implement some safety checks to see if your user has admin rights or not.

We should permit a "non-admin" install and have a fallback when the user doesn't have the rights to install python (e.g. install in user mode). How about this:
One click install becomes "two click" where the user is asked if they want to install in either admin or user mode. Admin mode can be greyed out if they're not an admin.

Hi @agvarghe - I tried replicating your steps but I couldn't find the bug... Please have a look at the attached vide showing my steps and replicating your operations. Could you share with me more information about your failure?
bug.mp4

When i try to click the install button after selecting the python version, there is no response

When i try to click the install button after selecting the python version, there is no response

Almost entirely sure this is due to permission issues.
@RobPasMue, let's ensure that we track errors and report them back to the user in a dialog. Looks like this is just getting locked up.

Yes, we will ensure that these errors are reported back, no problem. I will take care of the issue

After pair-programming with @agvarghe we saw that the main issue was that powershell.exe was not recognized as a command on his side. Once the C:\Windows\System32\WindowsPowerShell\ path was added to the Path system variable, everything started to work. So, there are a couple of things to be done with this issue:

[ ] Forward errors in a dialog box
[ ] Do not assume C:\Windows\System32\WindowsPowerShell\ is on Path. Instead of invoking powershell.exe, invoke the full path - Windows will always install it in that location.
[ ] Admin/user conditions require further investigation... I have the feeling we are already handling this - but following on @kmahajan-cadfem's comment, I'm not 100% confident. Could you open a CMD terminal or PowerShell terminal and run powershell.exe. Do you get any kind of error on your side when doing that? See this video

https://user-images.githubusercontent.com/37798125/234792109-3d453e23-f474-4ec6-8885-5797fefb034c.mp4

@RobPasMue :
I can run powershell without any changes to my PATH variable.
I did a deep dive in the code and found out following
on my machine, it fails because the download itself is not working
i have separated the code like following
import requests
import os
from appdirs import user_cache_dir

CACHE_DIR = user_cache_dir("ansys_python_installer")
selected_version = "3.10.0"
url = f"https://www.python.org/ftp/python/{selected_version}/python-{selected_version}-amd64.exe"
filename = f"python-{selected_version}-amd64.exe"

def download( url, filename, when_finished=None, auth=None):
    """Download a file with a progress bar.

    Checks cache first. If cached file exists and is the same size
    as the file to be downloade, uses cached file.

    ``when_finished`` must accept one parameter, the path of the file downloaded.

    Parameters
    ----------
    url : str
        File to download.

    filename : str
        The basename of the file to download.

    when_finished : callable, optional
        Function to call when complete. Function should accept one
        parameter: the full path of the file downloaded.

    auth : str, optional
        Authorization token for GitHub. This is used when
        downloading release artifacts from private/internal
        repositories.

    """
    request_headers = {"Accept": "application/octet-stream"}
    if auth:
        request_headers["Authorization"] = f"token {auth}"

    # initiate the download
    session = requests.Session()
    response = session.get(
        url, allow_redirects=True, stream=True, headers=request_headers
    )
    tsize = int(response.headers.get("Content-Length", 0))

    if response.status_code != 200:
        print(
            f"Unable to download {filename}.\n\nReceived {response.status_code} from {url}"
        )

        return

    output_path = os.path.join(CACHE_DIR, filename)
    if os.path.isfile(output_path) and tsize:
        print("%s exists at in %s", filename, CACHE_DIR)
        file_sz = os.path.getsize(output_path)
        if tsize == file_sz:
            print("Sizes match. Using cached file from %s", output_path)
            if when_finished is not None:
                print(output_path)
            return

        print("Sizes do not match. Ignoring cached file.")

download(url,filename)

and for above I get the following output
---------------------------------------------------------------------------
SSLCertVerificationError                  Traceback (most recent call last)
d:\users\mkanchan\pyansys_venvs\mytest\lib\site-packages\urllib3\connectionpool.py in urlopen(self, method, url, body, headers, retries, redirect, assert_same_host, timeout, pool_timeout, release_conn, chunked, body_pos, **response_kw)
    709                 headers=headers,
--> 710                 chunked=chunked,
    711             )

d:\users\mkanchan\pyansys_venvs\mytest\lib\site-packages\urllib3\connectionpool.py in _make_request(self, conn, method, url, timeout, chunked, **httplib_request_kw)
    385         try:
--> 386             self._validate_conn(conn)
    387         except (SocketTimeout, BaseSSLError) as e:

d:\users\mkanchan\pyansys_venvs\mytest\lib\site-packages\urllib3\connectionpool.py in _validate_conn(self, conn)
   1041         if not getattr(conn, "sock", None):  # AppEngine might not have  `.sock`
-> 1042             conn.connect()
   1043

d:\users\mkanchan\pyansys_venvs\mytest\lib\site-packages\urllib3\connection.py in connect(self)
    423             ssl_context=context,
--> 424             tls_in_tls=tls_in_tls,
    425         )

d:\users\mkanchan\pyansys_venvs\mytest\lib\site-packages\urllib3\util\ssl_.py in ssl_wrap_socket(sock, keyfile, certfile, cert_reqs, ca_certs, server_hostname, ssl_version, ciphers, ssl_context, ca_cert_dir, key_password, ca_cert_data, tls_in_tls)
    449         ssl_sock = _ssl_wrap_socket_impl(
--> 450             sock, context, tls_in_tls, server_hostname=server_hostname
    451         )

d:\users\mkanchan\pyansys_venvs\mytest\lib\site-packages\urllib3\util\ssl_.py in _ssl_wrap_socket_impl(sock, ssl_context, tls_in_tls, server_hostname)
    492     if server_hostname:
--> 493         return ssl_context.wrap_socket(sock, server_hostname=server_hostname)
    494     else:

C:\Program Files\ANSYS Inc\v231\commonfiles\CPython\3_7\winx64\Release\python\lib\ssl.py in wrap_socket(self, sock, server_side, do_handshake_on_connect, suppress_ragged_eofs, server_hostname, session)
    422             context=self,
--> 423             session=session
    424         )

C:\Program Files\ANSYS Inc\v231\commonfiles\CPython\3_7\winx64\Release\python\lib\ssl.py in _create(cls, sock, server_side, do_handshake_on_connect, suppress_ragged_eofs, server_hostname, context, session)
    869                         raise ValueError("do_handshake_on_connect should not be specified for non-blocking sockets")
--> 870                     self.do_handshake()
    871             except (OSError, ValueError):

C:\Program Files\ANSYS Inc\v231\commonfiles\CPython\3_7\winx64\Release\python\lib\ssl.py in do_handshake(self, block)
   1138                 self.settimeout(None)
-> 1139             self._sslobj.do_handshake()
   1140         finally:

SSLCertVerificationError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: self signed certificate in certificate chain (_ssl.c:1091)

During handling of the above exception, another exception occurred:

MaxRetryError                             Traceback (most recent call last)
d:\users\mkanchan\pyansys_venvs\mytest\lib\site-packages\requests\adapters.py in send(self, request, stream, timeout, verify, cert, proxies)
    498                     retries=self.max_retries,
--> 499                     timeout=timeout,
    500                 )

d:\users\mkanchan\pyansys_venvs\mytest\lib\site-packages\urllib3\connectionpool.py in urlopen(self, method, url, body, headers, retries, redirect, assert_same_host, timeout, pool_timeout, release_conn, chunked, body_pos, **response_kw)
    787             retries = retries.increment(
--> 788                 method, url, error=e, _pool=self, _stacktrace=sys.exc_info()[2]
    789             )

d:\users\mkanchan\pyansys_venvs\mytest\lib\site-packages\urllib3\util\retry.py in increment(self, method, url, response, error, _pool, _stacktrace)
    591         if new_retry.is_exhausted():
--> 592             raise MaxRetryError(_pool, url, error or ResponseError(cause))
    593

MaxRetryError: HTTPSConnectionPool(host='[www.python.org](https://www.python.org/)', port=443): Max retries exceeded with url: /ftp/python/3.10.0/python-3.10.0-amd64.exe (Caused by SSLError(SSLCertVerificationError(1, '[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: self signed certificate in certificate chain (_ssl.c:1091)')))

During handling of the above exception, another exception occurred:

SSLError                                  Traceback (most recent call last)
~\AppData\Local\Temp\ipykernel_11772\2224607365.py in <module>
----> 1 download(url,filename)

~\AppData\Local\Temp\ipykernel_11772\3956260789.py in download(url, filename, when_finished, auth)
     42     session = requests.Session()
     43     response = session.get(
---> 44         url, allow_redirects=True, stream=True, headers=request_headers
     45     )
     46     tsize = int(response.headers.get("Content-Length", 0))

d:\users\mkanchan\pyansys_venvs\mytest\lib\site-packages\requests\sessions.py in get(self, url, **kwargs)
    598
    599         kwargs.setdefault("allow_redirects", True)
--> 600         return self.request("GET", url, **kwargs)
    601
    602     def options(self, url, **kwargs):

d:\users\mkanchan\pyansys_venvs\mytest\lib\site-packages\requests\sessions.py in request(self, method, url, params, data, headers, cookies, files, auth, timeout, allow_redirects, proxies, hooks, stream, verify, cert, json)
    585         }
    586         send_kwargs.update(settings)
--> 587         resp = self.send(prep, **send_kwargs)
    588
    589         return resp

d:\users\mkanchan\pyansys_venvs\mytest\lib\site-packages\requests\sessions.py in send(self, request, **kwargs)
    699
    700         # Send the request
--> 701         r = adapter.send(request, **kwargs)
    702
    703         # Total elapsed time of the request (approximately)

d:\users\mkanchan\pyansys_venvs\mytest\lib\site-packages\requests\adapters.py in send(self, request, stream, timeout, verify, cert, proxies)
    561             if isinstance(e.reason, _SSLError):
    562                 # This branch is for urllib3 v1.22 and later.
--> 563                 raise SSLError(e, request=request)
    564
    565             raise ConnectionError(e, request=request)

SSLError: HTTPSConnectionPool(host='[www.python.org](https://www.python.org/)', port=443): Max retries exceeded with url: /ftp/python/3.10.0/python-3.10.0-amd64.exe (Caused by SSLError(SSLCertVerificationError(1, '[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: self signed certificate in certificate chain (_ssl.c:1091)')))

although downloading using the URL in browser works fine.

Interesting, so it's failing on download... and SSL Certificate verification. I'll try to investigate this. Thanks @kmahajan-cadfem!!
Also.. I find kind of weird this behavior:
C:\Program Files\ANSYS Inc\v231\commonfiles\CPython\3_7\winx64\Release\python\lib\ssl.py in wrap_socket(self, sock, server_side, do_handshake_on_connect, suppress_ragged_eofs, server_hostname, session)
...

Looks like you are accessing the Python Ansys unified install. Is this your pre-existing Python environment? This is unexpected behavior.

@RobPasMue : yes I am using pre-existing virtual env, we have some powershell scripts which can create virtual envs, similar to this tool, key difference is that we are reusing the Ansys CPython and dont have GUI.
one solution could be to just create a powershell web request, instead of python requests. (as fail safe)
Invoke-RestMethod 'https://www.python.org/ftp/python/3.10.0/python-3.10.0-amd64.exe' -Method 'GET'  -OutFile "python-3.10.0.exe"

or just use wget, should be available on windows as well under powershell.

Yes, that's right. I was thinking precisely on that solution as well. Just handling everything from powershell rather than delegating to requests - in case of failure. Let me work on that! I'll open a new issuw for this just do decouple them. Thanks @kmahajan-cadfem - we really appreciate all your feedback!


Animated Splash screen

This PR shows an animated splash screen during initialization phase. It is platform-independent (in contrast to launch4j) and also shows progress messages.
Unfortunately, LWJGL 2 does not allow for creating a hidden display window. As a consequence, the window still shows black for 1-2 seconds while shaders, etc. are loading. In LWJGL 3, window visibility can be toggled though.
Closes #1454

Tested out great now! I added a run config entry for IntelliJ with the splash screen and merged :-)


Xamarin Form CachedImage doesn't appear to honor the global cache duration

I set it in the Init config to be 90 days and it was always zero.  Set CachedImage CacheDuration in XAML and the correct duration was set on the temp file (looked at the temp path in a UWP app)
This doesn't seem like the intended behavior?

Seems like duplicate for https://github.com/luberda-molinet/FFImageLoading/issues/396

Your right, sorry I missed that!

Closed since it was a dup


NAs getting collected as FALSE

See https://stackoverflow.com/questions/47539670/convert-a-string-to-logical-in-r-with-sparklyr/47559040#47559040
Also
spark_lgl %>% mutate_if(is.character, as.logical) %>% transmute(ac =  as.character(a), bc = as.character(b))
Applying predicate on the first 100 rows
# Source:   lazy query [?? x 2]
# Database: spark_connection
     ac    bc
  <chr> <chr>
1  true  true
2  true  true
3  <NA>  <NA>
4  <NA>  <NA>
5 false false
6 false false

Tracked in https://github.com/rstudio/sparklyr/issues/941


Update CONTRIBUTORS.md

Codecov Report

Merging #2719 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #2719   +/-   ##
=========================================
  Coverage     42.84%   42.84%
  Complexity      185      185
=========================================
  Files           150      150
  Lines         11306    11306
  Branches       1139     1139
=========================================
  Hits           4844     4844
  Misses         6137     6137
  Partials        325      325

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55ba362...a5c615e. Read the comment docs.


Force blockly svg resize if container is resized.

This PR fixes two issues:

window is resized when blockly tab is not showing. When user returns to blockly tab, blockly div has wrong dimensions (0x0) and does not display content
in certain cases where the browser screen changes size (such as using fullscreen button, showing/hiding dev tools, maximizing browser, etc.), the blockly div has wrong dimensions and does not take up full container space.

To fix, we monitor the container div size.  If it changes, call Blockly.svgResize which resizes the blockly container div to fit our parent div.  The only side effect that I've seen so far is that we might lose the pan when switching tabs, resizing, and returning to the blockly tab.

@sfentress I think it was here in the Blockly docs that gave me the big clue:
https://developers.google.com/blockly/guides/configure/web/resizable


Fix/build setup

This PR allows us to write typescript & makes it easier to setup node debugging. JS/TS is compiled to .compiled now with source maps when running yarn watch and without for yarn build

looks great! .compiled is a bit awkward, can we just use dist? i wonder if the default eslint/babel/etc configs expect the standard typescript output directory?

Sure thing :) one sec

Updated it!


Example with no replication needed

In the above section "Create an image version", you provide an example that includes replicating to other datacenters. Most people will want to test this out by keeping it as simple as possible. While your example would be "best case", it would be good to include an example that creates an image version with the one image in its current datacenter.
Thanks!

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: 1301ad5d-8a13-402e-aedb-2a77272ad042
Version Independent ID: 83cd183c-c143-8bac-3b15-34ec2f887456
Content: Create shared VM images with Azure PowerShell
Content Source: articles/virtual-machines/windows/shared-images.md
Service: virtual-machines-windows
GitHub Login: @cynthn
Microsoft Alias: cynthn

Thanks for the feedback! I have assigned the issue to the content author to investigate further and update the document as appropriate.

#please-close The docs for Shared Image Gallery have been rewritten lately. I think the examples still give multiple replication targets, because it can be hard to figure out the syntax for more than one target.


unable to build because of libtls dependency

using fedora 36
tried installing https://fedora.pkgs.org/36/rpm-sphere-x86_64/libtls19-3.4.2-1.x86_64.rpm.html didn't help
also already have libretls installed

needed this https://packages.fedoraproject.org/pkgs/libretls/libretls-devel/


Adding template support to python client build

Adding setup.mustache and configuring Makefile to use it and to not delete it.
Done to allow to override of the long_description field to allow fix of #2344
We should consider upgrading our OpenAPI Generator!!

Code was merged as part of #4260


Change Tooling article content layout

Fixes #32540
Currently, this is set up to change from pivoting on platform to pivoting on tooling (VS, VSC, CLI).

Internal previews

📄 File
🔗 Preview link

aspnetcore/blazor/tooling.md
Tooling for ASP.NET Core Blazor

Something else occurs to me. These days, this article relies on several terms and concepts that are introduced by the Fundamentals > Overview article. This whole article could appear immediately after Fundamentals > Overview in the Fundamentals node to make sure that when devs are coming into this that they at least get that little bit of introduction to the terms and concepts that this article uses (and the cross-links that are here now to that article could be removed). The downside is that the Tooling article is then buried under a node that devs need to open to find it and not right out in the root of the Blazor ToC. Let me know if you think the Tooling article should be moved on those grounds.

@danroth27 ... Whew! 😅 ... You'll probably need to start over and re-review the entire thing because a lot changed on that commit.
After this is all squared away, I'll get right over to the Blazor tutorial PR and make the same updates there WRT the specific tooling gestures (e.g., dotnetcli over powershell, the use of the Command Palette over .NET CLI commands for VSC, etc.). I'll get that done quickly (by EOD probably), and then you'll be able to review that PR with the latest approaches.
I opened (P2) ...
Blazor CLI commands that run apps moving to dotnet watch
https://github.com/dotnet/AspNetCore.Docs/issues/32845
I also opened (P2) ...
Blazor VSC guidance to adopt Command Palette build and start gestures
https://github.com/dotnet/AspNetCore.Docs/issues/32846

I'm still not sure about this, but I'm removing it. It doesn't seem to be required any longer, but the command is still there to create the assets.
Select **View** > **Command Palette** and type "`.NET`" into the search box. From the list of commands, select the "`.NET: Generate Assets for Build and Debug`" command.

Anything else? ... Are you still reviewing?

Anything else? ... Are you still reviewing?

Yeah, I still need to go through the CLI stuff.

I'm going OOF ... grocery store 🥫. I'll be back later, but I'll have my phone with me to answer any ❓ that may arise.


Add IFUM token

You have not upload the token profile doucument, which is ended with'.json'.
Check it and read the 'README' tutorial for more details.

I'd uploaded json file.
Please check it out. Thank you.

Project Introduction
INFLEUM is executed by ELSOLCOM, a company that specializes in growing start-up’s and SMEs.
ELSOLCOM, through its internal support and collaborative efforts, has established diverse references
and clientele that are undergoing a large scale of qualitative and quantitative prosperity.
Henceforth, through the decentralized INFLEUM Platform, businesses will overcome regional/physical
obstacles, and will be able to provide advertisement/marketing services specific to diverse clients’
needs. By inducing active competition of services among INFLEUM users, there will be a boom in
services of high quality and quantity with minimized costs, and eventually, the project will be able to
reach economy of scale. Therefore, the ecosystem will be bigger and its worth will multiply with user
growth, simultaneously boosting the user profits.
The INFLEUM Platform utilizes Smart Contract and Blockchain to transparently provide information on
advertisement/marketing client and consumer, activities and results of users behind projects, creation
and ownership details, and more through the Distributed Ledger. In other words, all activities of users can be tracked and verified, thus preventing centralized distortion. The platform provides a completely
objective measurement system, which depends on credibility of results and fair assessment of
participation.
The INFLUEM Platform aims to be an Open Innovation Platform as it considers the establishment
of a decentralized ecosystem to be significant for all users–the enterprises and individuals. Therefore,
an environment will be created where more diverse and distinguished third party DAPPs, aside from
the previously installed DAPPs, can be developed and expanded by utilizing open API/SDK,
consequently allowing easy access and application
Official Website:
https://infleum.io
Etherscan
https://etherscan.io/token/0x5278852029f4fdf91eb2d40614ff49b4b4b7c765
Gas Limit:
60000
Token icon:

I've merged the pr. You can recommend imToken 2.0 to your token holder to manage IFUM.

I have confirmed that IFUM is registered in imToken2.0.
Thank you.


[Image][Performance] Fast Image with Auth Token Header Caching (revert follow-up)

Details

This PR is the follow up the the original fast image PR that was reverted here. The key improvements in this PR are:

[x] Better naming convention for the Image component
[x] Getting the auth token and headers are now fully encapsulated by the Image component itself (+ reactive with the withOnyx HoC rather than Onyx.connect())
[x] Resolves the revert issue https://github.com/Expensify/App/issues/13002 where react-native-fast-image would not load if the style width / height were set to zero.

Fixed Issues

Original image caching issue - https://github.com/Expensify/App/issues/10792
Android PDF thumbnail not displayed (triggered initial revert) - https://github.com/Expensify/App/issues/13002
Tests

Checkout this branch & npm i
Run the app on both iOS and Android
Open a chat with some image attachments & PDF thumbnails
They will load in as normal the first time, but reload the app and then you will see the cached images loaded instantly in the same chat

[x] Verify that no errors appear in the JS console

Offline tests

Open a chat and wait for all chat attachment thumbnail images to finish loading
Go offline and reload the app and open the same chat - all chat attachment thumbnails should be displayed
Go back online and open the attachment view of an image attachment in the chat and let it load
Go offline, reload the app and open the same chat and then open the same attachment - this should very briefly show a loading spinner before then displaying the cached image

NOTE: The thumbnail images should be cached for any attachment type, however, when opening the attachment view for an attachment only image attachments will be cached - for example, PDFs will have their preview thumbnail image cached, but the PDF file itself is not cached and thus it is expected behaviour that you will see "Failed to load PDF" when performing steps 3&4 for PDF attachment.
QA Steps

[ ] Open a report chat and add a new image attachment and send so the image appears in the chat (this should display a loading indicator the first time it is requested)
[ ] Reload the app and you should see the cached image displayed (with no loading indicator)
[ ] Click on an image in the chat to open the attachment modal view, close it and reopen it to see the cached image displayed without a loading state
[ ] Verify that any attachments still download as normal without any errors (including image attachments and PDFs)
[ ] Upload a PDF and the thumbnail should render correctly in chat

PR Author Checklist

[x] I linked the correct issue in the ### Fixed Issues section above
[x] I wrote clear testing steps that cover the changes made in this PR

[x] I added steps for local testing in the Tests section
[x] I added steps for the expected offline behavior in the Offline steps section
[x] I added steps for Staging and/or Production testing in the QA steps section
[x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
[x] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).

[x] I included screenshots or videos for tests on all platforms
[x] I ran the tests on all platforms & verified they passed on:

[x] Android / native
[x] Android / Chrome
[x] iOS / native
[x] iOS / Safari
[x] MacOS / Chrome / Safari
[x] MacOS / Desktop

[x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
[x] I followed proper code patterns (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I followed the guidelines as stated in the Review Guidelines
[x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] I verified that if a function's arguments changed that all usages have also been updated correctly
[x] If a new component is created I verified that:

[x] A similar component doesn't exist in the codebase
[x] All props are defined accurately and each prop has a /** comment above it */
[x] The file is named correctly
[x] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[x] The only data being stored in the state is data necessary for rendering and nothing else
[x] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[x] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[x] All JSX used for rendering exists in the render method
[x] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[x] If any new file was added I verified that:

[x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)

[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop
https://user-images.githubusercontent.com/31568400/210455348-0b372b0a-5ca2-472c-a200-971a05656baa.mov

iOS
https://user-images.githubusercontent.com/31568400/210451867-a91623f3-1352-49a1-9190-b3a40fa2e3bf.mp4

Android
https://user-images.githubusercontent.com/31568400/210453901-cda6f127-e509-4de6-8909-eb106cfaa236.mov

I've just done a quick test with the fix that was mentioned here in slack. It does indeed load the images and the onLoad event is now fired, however as suggested by other comments in the OG GitHub issue, this applies no compression to the actual image on Android and thus large images cause a crash - I tested with a 10,000 x 5,000 px image (only 1.1mb on disk) and was able to reproduce this.
Test image: https://user-images.githubusercontent.com/31568400/205428950-cd7119fb-cc70-4e76-9518-8027ead43f6f.jpg
I will do some more digging and see if a better fix can be found as we definitely cannot apply this directly if it is going to cause crashes. Also maybe raises a question if we want to ever impose a max resolution for images and videos uploaded as chat attachments cc @Beamanator. Will keep you posted 🙌

Looks like Glide (the image loading module used in Fast Image on Android) requires a valid, non-zero view size before it can begin image loading as this comment from one of the collaborators suggests:

https://github.com/bumptech/glide/issues/618#issuecomment-275277165
https://github.com/bumptech/glide/blob/v4.7.1/library/src/main/java/com/bumptech/glide/request/target/ViewTarget.java#L516
https://bumptech.github.io/glide/doc/targets.html#view-targets

With that in mind do we still want to class the onLoad not firing for zero width/height images on Android as a bug or just how it should work on Android? I know you strive to have the same functionality of components cross-platform
I think the options are from here:

Rework Glide so it can somehow provide the width/height of the image source and fire the onLoad event regardless of view size
Rework Fast Image to have either a onImageSizeAvailable prop at the component level or a static FastImage.getSizeWithHeaders() function that can somehow access the natural width and height of the image with Glide, likely through Targets.SIZE_ORIGINAL
Ignore the fact it doesn't load if the view size is zero and then always set the expectation that the view should have a non-zero width/height (which in this specific case will be fixed by the backend change documented here by @Beamanator https://github.com/Expensify/App/pull/12648#issuecomment-1335180031)

Let me know your thoughts @Beamanator @kidroca 🙌

Hey @thomas-coldwell thanks so much for the in depth investigation!! I believe I'd prefer we go with options 1 or 2 ^ since they're fixing bugs instead of implementing workarounds... It looks like https://github.com/bumptech/glide/ is a pretty active repo so maybe we could get the fix in there, though that issue you linked was opened (and closed) a long time ago 😅  Maybe you can see if there's other related issues that are more recent / still open? And if not, maybe we can start a discussion there to see if it's something they'd consider "fixing"?

Actually, this seems to be the recommended way of getting the natural dimensions of the image with Glide and ensure that it is only making a single request or decoding this from the cached image https://github.com/bumptech/glide/issues/781#issuecomment-160953996.
One question I have is do we want to always report the natural image dimensions in the onLoad event (which would be the case with this fix)? Glide currently resizes images so they have a smaller memory footprint and then reports the resized dimensions of the image like so (this is for a tiny 50x50 dp view):
 LOG  RN Image (onLoad) = {
  "height": 1800,
  "width": 1800,
}
 LOG  Fast Image (onLoad) = {
  "width": 132,
  "height": 132
}

I'm assuming we do want the natural dimensions so the API is identical with RN core image component? I'm conscious that the image displayed by glide will not actually have these dimensions but I don't think this will have any knock on effects as the width and height are just used to compute image aspect ratio and not directly for other view layouts.

@thomas-coldwell since there's no current issues / PRs on Glide about this issue, do you think it makes sense to open an issue there to discuss this situation? It's possible those maintainers would recommend we take this issue to the react-native-fast-image lib, but I'm inclined to want to start there since that's higher up the chain, what do you think?
Also I am not sure who we'd contact at @pieter-pot/react-native-fast-image to get a PR merged... I wonder if we would have to go through the main react-native-fast-image lib first and get that passed down. Though, as I'm comparing the two, there's very very very small differences between the fork we use & the main library - honestly it's hard to even see what the differences are, do you see any? I wonder if it's a better idea to see if we can use the main repo instead of the fork?
About the natural image dimensions vs resized dimensions, this is getting a bit far above my understanding, sorry 😅  So far I like your thinking but honestly I don't quite understand the benefits / drawbacks very well  - if you think you're onto something I thinkkkk we should move forward with that and see what library maintainers think and move forward that way?
Also FYI I'm going to start a discussion in Slack (soon) to see if people think we should move forward with a little workaround while this conversation / implementation is happening in other repos, then once the fix is implemented & is usable by the New Expensify app, we can remove the workaround. Coming soon :D

Hey @Beamanator 👋 I don't think it makes sense to open an issue with Glide, glide works as it should loading an image into a target - usually a drawable. This is how Glide is used in Fast Image.
The fix that I've found, and implemented in a separate sandbox app, is to setup an additional request for glide which decodes the image to get the size and then emits this as the onLoad event back to JS.
I agree there seems to be literally no difference between the fork we are currently using and the main react-native-fast-image repo so I'd opt that we make a fork of react-native-fast-image for Expensify and then we can hopefully PR these changes upstream (though the maintainer of Fast Image is very inactive so the changes might have to stay on our fork, but will try to get it merged upstream nonetheless). Is this possible to get a fork on Expensify or should I just fork it myself? 😃
The natural dimensions are that of the source image we load from the backend e.g. you might have a 1000x1000px image. With Glide and a lot of other image loading libraries they will resize this down appropriately for the target e.g. if its being loaded into a tiny 50x50dp view (like a profile picture) then it will resize it down to maybe 100x100px to preserve memory. I think it makes more sense to do it the same as the RN core Image API and report the natural dimensions so will stick with that 👍

The fix that I've found, and implemented in a separate sandbox app, is to setup an additional request for glide which decodes the image to get the size and then emits this as the onLoad event back to JS.

That sounnnds great! Do you think we should try to test it with our App before moving forward with that solution? Or was your test solid enough? :D
About the forking question - we reeeeally try not to make our own Forks whenever possible, so what we'll most likely do is try to get the conversation & PR going on the main react-native-fast-image repo, and if it goes too long w/ no response we will consider making the Fork. In the meantime I may implement the backend workaround just to get this unstuck

Perfect, cheers @Beamanator !! I'll create a fork and apply my changes there, open up a PR to the OG repo and then I'll test using my PR branch in the Expensify app as well to make sure there are no obvious issues with this approach 🙌

That sounds fantastic, thanks so much @thomas-coldwell ! 👍

I've fixed any merge conflicts there were (as main contained the revert changes for the original PR) so this should be good to test with the backend workaround if you wanted to @Beamanator :)
I'm now creating a PR for react-native-fast-image with the changes I made to return the natural dimensions from the onLoad event. There was one similar PR already opened there, but they decode the whole bitmap to get the size and use the .override(Target.SIZE_ORIGINAL) method for Glide which does not perform any compression and causes crashes on Android (I've verified this is the case in the Expensify app too with a 10,000px image) - their PR seemed to go stale due to these issues being pointed out. My PR is following the recommended way to achieve this as mentioned here https://github.com/bumptech/glide/issues/781#issuecomment-160953996
I will link this here when it's up 🙂

Thanks @thomas-coldwell ! I think I'll wait till some resolution on this convo (feel free to add your opinion) before testing w/ my workaround PR :D

Opened up the PR on the react-native-fast-image repo https://github.com/DylanVann/react-native-fast-image/pull/953 🙌

Reached out to maintainer to see if we can get the upstream PR moving - if no response this week we will have to think about forking

Established with @puneetlath that a patch of the upstream PR will work best for now and works perfectly fine with the native code changes. The upstream PR will remain open and hopefully the maintainer will have some time to eventually look at it - if it does get merged upstream this patch can then be removed and upgraded to the latest version

@kidroca might be good to get your eyes on this as you had comments on the previous PR :)

This is a rather big one :D
Shouldn't this have a C+?

Going through the comments and the code. (also from the previous PR before getting into the testing). Will have the review and testing completed by tomorrow.

Having some issues with sending attachments locally

I see many of the styling comments raised already hence I've added only one that wasn't added in the previous reviews. Still reviewing. Might need 1 more day to review and test.

Chatted with @mountiny on slack here and looks like this above error comes from the attachment picker process and not the changes in this PR - will do another PR to investigate this issue (or check if anyone has opened an issue for it already) 🙌

I am close to 70% done, will start testing in an hour or so. Should have all the screencasts in 2 hours.

@thomas-coldwell Is this an acceptable behavior when I open the image it shows a loader everytime? for iOS and Android?
https://user-images.githubusercontent.com/3069065/210849653-305858ca-d81a-42e5-974e-03849e3f5963.mov
https://user-images.githubusercontent.com/3069065/210849756-6a8fe0a4-c4d7-4ede-b4fc-90ee29341d1b.mov

Hey @mananjadhav there should be a very small delay where it is showing the loading indicator as it has to to perform some layout calculations after the image has already loaded as this is required by the parent ImageZoom component.

But on iOS some of those recordings seem to take a fairly long time so I will take a look and confirm there it is definitely getting the image from cache in this situation

I've made that change - was a lot more simple than I thought. The onProgress is only called if it is downloading an image and not from cache so now the loading state is only set if it actually is downloading and not getting it from the cache:
https://user-images.githubusercontent.com/31568400/210869498-40edbaef-7fc5-4794-9ad0-521059f2588e.mp4

Also just checked the other case where the loading indicator is shown when you have uploaded an image, it is displayed in the chat and then you got onto the attachment view and it appears to reload. The cache it still working correctly here its simply that the thumbnail URL differs with a .1024.jpg append and the image in the attachment view does not have this in the URL and thus they are separate images so it will have to re-download it and then subsequent visits to this image in the attachment view will show the cached copy.
Just wanted to document this in case there was any questions around this other loading state 🙌

Thanks for the updates & explanations @thomas-coldwell 👍 👍  I'll do another review shortly 🙏

@thomas-coldwell I've two comments on the testing:

When I upload and view smaller image, then it resizes on the attachment modal. This is an issue with iOS and Android.

https://user-images.githubusercontent.com/3069065/211095672-fe05df89-8508-4680-86e6-f1cf67b927c9.mov

Not sure if this is an issue, but on Android when I open the image in the attachment modal, I see a delay in the loading indicator. Are you seeing the same behavior?

https://user-images.githubusercontent.com/3069065/211096039-47ecaf5d-41bb-46fa-8c17-37018ed4a30e.mov

@Beamanator @puneetlath I've reviewed and tested this for the following:

Small, regular and Large Images
Offline loading state
PDF and Password Protected PDF
Other attachment types
File and Image Downloads

Do we have any regression suite that can validate if we need to cover something

Thanks @mananjadhav! I believe the second issues is caused by changing isLoading logic. Looks like I need to add a check in the onLoadStart to see if the image is coming from cache or not and then only set isLoading to true if Glide / SDWebImage is downloading it as otherwise there will always be a loading indicator. The first issue is related to this so will make a change and resolve both 🙌

Hey @mananjadhav this should be ready for a review again and you should see those two issues from the last review resolved now, cheers 🙌

Thanks @thomas-coldwell. I’ll be reviewing it today.

Hi @thomas-coldwell,
The earlier issue of image getting resized on view is resolved.
But now a flicker is introduced with the last change. You'll notice a small flicker, where actually the image is previewed, then a loader is shown and then the image loads when it is already cached. This issue is only with iOS and not with Android. . Are you seeing this? Attaching the screencast below.
https://user-images.githubusercontent.com/3069065/211210885-eeefb512-c08b-4d56-babe-907395d2d301.mov

Though on Android I have a question, will it always a show a loader even when the image is cached? It shows a loader very quick, where I am assuming RNFastImage calculates the image size?
https://user-images.githubusercontent.com/3069065/211210972-e24802d9-d684-4383-ab18-f8da8e5573b6.mov

@thomas-coldwell Quick bump on the comments above.

Thanks @mananjadhav - yeah I think this problem is being caused by the ImageZoom component as that requires the width/height of the image, but this is only available once the image has loaded as thus there will always be some small flicker. At the minute the image is hidden until the image dimensions are set for the ImageZoom which is what causes the flicker - ideally we would need to replace the ImageZoom component so that it does not require knowledge of the image dimensions OR we just always show some loading state (whether that's showing the indicator or just hiding the image) while its performing this layout calculation. What are your thoughts @Beamanator ? I think this is probably linked to https://github.com/Expensify/App/issues/12893

@thomas-coldwell Yeah ideally we won't have the flicker where it goes from image to loader to image (which I think is currently only happening on iOS). So I agree it would be best to show the loader while layout calculation is happening - is this something easily done in this PR?

Alright, I've updated this so it always shows the loading spinner for actual image load + layout calculation.
In a follow-up PR (or the PR for https://github.com/Expensify/App/issues/12893) this can ideally be changed so it only shows a loading state when the image is loading and more specifically only when its having to download the image the first time to cache. As a result I've left the code that supplies the cachePath in the onLoad event for any follow up work 🙂

Also just to double check, I've made the initial width/height zero in the ImageView on native - I had left a comment before that this should use large values to avoid the ImageZoom component causing lower resolutions images to be displayed but I've tested with some high resolution images and it doesn't seem to cause any of these issues on either platform.
Let me know if you have any concerns about this or if you know it will likely cause any kind of regression here that I'm not aware of :)

@thomas-coldwell thanks for the updates on this!
@mananjadhav Will you be able to retest this based on the updated behaviour?

@mountiny I saw the update last night, but I wasn't sure if we agreed with the behavior. Can you or @Beamanator confirm if the approach is okay then I can test today and close it.

I didn't reach the offline tests for Android/iOS. Will test and update.

@Beamanator I believe that is the expected behaviour for a PDF when offline as PDFs are not cached - only image attachments. Though I believe that would certainly be a good follow-up PR to add full chat attachment cache support

Aah ok @thomas-coldwell so that would probably be the same for all non-image attachments i guess? It would be great to fix that in a folow up :D

Yeah all caching is handled internally by Fast Image so we'd need a separate mechanism for this non-image cache 👍

Aah right good point @thomas-coldwell - that's out of scope for now 😅

Reviewer Checklist

[x] I have verified the author checklist is complete (all boxes are checked off).
[x] I verified the correct issue is linked in the ### Fixed Issues section above
[x] I verified testing steps are clear and they cover the changes made in this PR

[x] I verified the steps for local testing are in the Tests section
[x] I verified the steps for Staging and/or Production testing are in the QA steps section
[x] I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)

[x] I checked that screenshots or videos are included for tests on all platforms
[x] I included screenshots or videos for tests on all platforms
[x] I verified tests pass on all platforms & I tested again on:

[x] Android / native
[x] Android / Chrome
[x] iOS / native
[x] iOS / Safari
[x] MacOS / Chrome / Safari
[x] MacOS / Desktop

[x] If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
[x] I verified proper code patterns were followed (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I verified that this PR follows the guidelines as stated in the Review Guidelines
[x] I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] If a new component is created I verified that:

[x] A similar component doesn't exist in the codebase
[x] All props are defined accurately and each prop has a /** comment above it */
[x] The file is named correctly
[x] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[x] The only data being stored in the state is data necessary for rendering and nothing else
[x] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[x] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[x] All JSX used for rendering exists in the render method
[x] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[x] If any new file was added I verified that:

[x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)

[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web
https://user-images.githubusercontent.com/3069065/212072746-70b081c6-3fad-41ca-a637-f54ce1f009bd.mp4

Mobile Web - Chrome
https://user-images.githubusercontent.com/3069065/212071367-30e58556-f80a-4dbd-9f02-c7f7513dc103.mov

Mobile Web - Safari
https://user-images.githubusercontent.com/3069065/212071516-e51d1188-46ce-4e5c-80e0-8e87ad40a452.mov

Desktop
https://user-images.githubusercontent.com/3069065/212071776-19f42769-60e8-4a3b-8825-8c6ef23d214a.mov

iOS
https://user-images.githubusercontent.com/3069065/212071991-da5cb57d-215c-4343-bf78-dfcf0d647157.mov

Android
https://user-images.githubusercontent.com/3069065/212072304-45ebe8cb-3197-465f-a908-9d4f6bb3a702.mp4

@Beamanator Based on comments with @thomas-coldwell, I am ignoring offline PDF.  Force offline doesn't work as expected it seems and hence the PDF loaded for me. I turned off the Wifi on the emulator to reproduce the failed offline behavior. Also, even if I turn off Mac Wifi, the iOS Simulator still loaded the PDF (not sure how).
I also think we should add this as a Tests so that QA is on the same page.

@thomas-coldwell do you mind updating the Offline test a bit in your OP? @mananjadhav makes a good point that we want QA to be 100% on the same page as what we've decided, and right now I don't think the offline test case is very clear what we expect to happen

Thanks @Beamanator - I've updated the OP to reflect this - very good points for them to be aware of!

Looks fantastic now, thanks so much @thomas-coldwell 👍

@Beamanator @thomas-coldwell @puneetlath Anything pending here?

I think we were waiting to get one last review from @kidroca - I'll ping them on slack

I've tested (rebased) these changes with my PR for Web Images With Headers and they are working fine

Hey @kidroca thanks for the testing with your RNW PR! Agree on the comment on ImageWithSizeCalculation - the onLoad event for Fast Image will only be called if the image is successfully downloaded or retrieved from cache with a valid width/height, else it will call its onError event.
With that in mind I don't think we actually need the check here and it can just be removed 👍

Yeah I agree. I am going to test this in a while @Beamanator. Give me like 30-45 mins.

Amazing, thanks so much @mananjadhav 👍

Amazing @mananjadhav

@thomas-coldwell , @Beamanator
Quick update, I just tested Desktop and Web. The caching seems to be working fine. I won't be able to do offline tests as I can't disconnect my laptop from Wifi right now. While I am testing other platforms, I found one issue.
I missed to check this earlier. When I download the images and PDF, the file name of the downloaded file is correct. But when I download a word document, the file name of the downloaded doc, doesn't match the original file name. Attaching the video.
https://user-images.githubusercontent.com/3069065/212859216-36a86db4-3db8-46ac-b45b-3c24f97d894a.mov

@thomas-coldwell @Beamanator So I did all the testing for all the platforms and the caching works fine.
I've got two concerns/issues:

File Download name isn't working as expected so we'll have to fix that (which I posted in the previous comment).

If I am offline, and I open a new image, what's the expected output for the Attachment Preview? Android has just a loader, and Web has a blank screen. PDF shows Failed to load PDF file..  And I think that's how it is in the current prod/main version too.

https://user-images.githubusercontent.com/3069065/212867354-77212604-7f55-4d53-90a7-f7f4dbc2739e.mov

Thanks for the tests and notes @mananjadhav !
In my opinion:

File Download - I would be completely fine opening a follow-up issue to investigate this, I don't see how this PR could have broken that, especially if it's only for word documents
As for chat attachment images in the Attachment Preview - I think we'd expect to be able to see the images, right @thomas-coldwell ?

Note: For PDFs, we don't expect to see the PDF (see notes starting here)

Okay for 1.
for 2, what I meant is opening the image for the first time in the offline mode. I am inclined to solve this also as a follow up Pr because the behavior is same as prod, but just wanted to highlight it here.

@mananjadhav Aah because the image hasn't been viewed yet so it probably hasn't been cached either? I'd at least expect a consistent thing to happen across apps - but yeah as @thomas-coldwell said if this is consistent with production, let's discuss outside this PR - probably in Slack to get some more eyes 👍

Thanks for confirming @thomas-coldwell.
Agreed to keep both of them outside the scope of this PR. All good here then.

FYI it looks like the failing test is not related to this PR, and it was noted in Slack here: https://expensify.slack.com/archives/C01GTK53T8Q/p1673916481148719
So I'm going to merge 👍

Mentioned above that 1 test wasn't passing, but it's not related to this PR and is being investigated in Slack and has this issue for investigating: https://github.com/Expensify/App/issues/14352

🚀 Deployed to staging by @Beamanator in version: 1.2.56-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

🚀 Deployed to production by https://github.com/AndrewGable in version: 1.2.56-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅


Docs: add OLM install procedure

Missing information about how to install EnMasse using the OperatorHub

PR: https://github.com/EnMasseProject/enmasse/pull/3323

PR merged, so closing


Show options to build vsproj only if selected platform is windows

The description of what PR does and the PR itself are smaller than description how this PR done :joy:
Calling env["vsproj"] will result in error if you didn't add the option to variables. I mean if you didn't declare BoolVariable("vsproj", "Generate a Visual Studio solution", False) which won't declared on non-windows platforms after this PR
Python has short circuited bool operators so env["vsproj"] won't be even evaluated if "vsproj" in env returned false
Why not to use just "vsproj" in env? :joy: :joy: :joy: Because SCons is very stupidly designed... It was a surprise that in operator worked at all. But not as you would expect. :joy: You would expect it to tell you if it contains pair key, value with given key. Just like in normal dictionary...
But not it will actually tell you if you added "validator". I mean this declaration opts.Add(BoolVariable("threads", "Enable threading support", True)).
P.S: I am now sick and it is hard to think maybe two previous paragraphs are wrong not by itself but a bit unrelated to this PR

As far as I know, one of the features was being able to call vsproj from linux or mac. Does this perserve that?

As far as I know, one of the features was being able to call vsproj from linux or mac. Does this preserve that?

I am not sure what do you mean. Is there a feature to build godot with vsproj on linux or mac or you are about just generating vsproj file on linux and macos?
If the second so it is preserved. The only difference that now you need to explicitly add that your target platform is windows scons p="windows" vsproj="yes" but you can still do it on linux and mac. I have linux and tested it before and after PR. It works. But linux/mac users won't be annoyed by vsproj options in their scons -h output
If there is actual building from generated file on linux or mac, I know about it only now and don't know maybe it is preserved ¯\_(ツ)_/¯

CC @shana (too many cc for one person in several minutes :sweat: )
I think this brakes something intended?

I don't think this is useful? The option is there in order to generate a VS solution and projects files that can be loaded into IDEs that support the format. You can generate those project files from any platform, not just windows - and technically if an IDE supports the format, it can load them regardless of whether it's on windows or not, the .sln/.vcxproj formats aren't Windows-specific.
Additionally, having the option only on the Windows platform doesn't make sense - you can generate VS projects for all platforms that you can build on Windows. This includes Android out of the box, but also all console platforms.

To be clearer, because the word "Windows" and "Platform" are a bit overloaded: on this laptop that is running Windows 11 right now, I can do the following to generate VS project files:

scons vsproj=yes : Generates project files to build the editor, assuming platform=windows
scons platform=android target=template_debug vsproj=yes : Generates project files to build the debug template for Android
scons platform=gdk target=template_debug vsproj=yes : Generates project files to build the debug template for the custom GDK platform that I have added to my godot source locally

As you see, that only works because the vsproj option is available for all godot target platforms. If it were only defined on the windows platform, it would not be possible to generate project files for any other godot target platform.


Atmo docs revamp

Fixed homepage icons and added new Sat icon
Moving to Docusaurus built-ins
Further improved sidebar
Added first take on Sat docs
Revamped & slightly reorganized Atmo docs

Preview: https://suborbital.github.io/docs/

This is awesome! A few notes:

Should we remove the logo from the top bar on the homepage? Seems odd to have two
IMO the ordering for the Runnable API section should be: Example runnable, handling requests, modifying responses, logging, http, graphql, authentication, cache, files
The warning about Subo on Windows is a bit too harsh, could we downgrade from a yellow warning to an 'info', and change the wording to say "not officially supported" rather than "not compatible" ?
And could we move the Using SQL databases page down to the Runnable API section, and add some more languages? It's supported in more than just Rust now 🙂

I filed #28 for a follow-up for the logo issue.
Good point on the Runnable API, I reorganized it and moved the example runnable into the intro page. I fixed the outstanding language-specific APIs there too. I also have #27 for making this easier (potentially automated?) in the future, because it's annoying to maintain this manually, and error-prone (same as why we want wit-bindgen).
I adjusted the Windows support note, the SQL stuff now lives in the Runnable API now too (this was something I was wondering about myself before), I'll look into AS/Swift support and can add that note later as well.

This is wonderful! There are only two small tweaks I'd like to make before merging this PR:

I've changed Atmo's description to be 'Application framework for cloud native WebAssembly', so let's try using that on the homepage here as well

I've changed Sat's description to 'Tiny & fast WebAssembly edge compute server` so let's use that

I've also filed https://github.com/suborbital/docs/issues/30 with all of my nitpicks and tweaks, but this PR can proceed without those :)

Love the new taglines! Adjusted them accordingly, and will merge this now.


select all without keywords will break things / confuse users

fixed by https://github.com/nance-eth/nance-interface/commit/bce2fac79e9f30bff64b1f7990edd2cde7bf990e