[HOLD #12603][Image][$1000] Improve NewDot image uploading experience

If you haven’t already, check out our contributing guidelines for onboarding and email<EMAIL_ADDRESS>to request to join our Slack channel!

Action Performed:

Upload an image in App

Expected Result:
The image should be grayed (opacity 0.6 i.e., chatItemUnsentMessage) until its uploaded and once its uploaded it should not show the white thumbnail with loader

and instead continue showing the local thumbnail or show the final image immediately.
Actual Result:

The local image is being uploaded and we see the local thumbnail,

The local image has finished uploading and now we attempt to render it which first shows up a white thumbnail with loader

The render completes and we see the uploaded image

Workaround:
Can the user still use Expensify without this being fixed? Have you informed them of the workaround?
Platform:

Web
iOS
Android
Desktop App
Mobile Web

Version Number: 1.1.76
Reproducible in staging?: Y
Reproducible in production?: Y
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1654262165566879
View all open jobs on GitHub
cc @michaelhaxhiu, @cead22, @kidroca.

Good change. I agree.

@JosueEchandiaAsto Thanks for the proposal. But as you are a new contributor and have already been assigned two 2 tasks, you will have to wait before applying to new tasks.
Contribution policy: https://github.com/Expensify/App/blob/main/CONTRIBUTING.md#payment-for-contributions

New contributors are limited to working on one job at a time, however experienced contributors may work on numerous jobs simultaneously.

@parasharrajat Both tasks have already been completed and have been merged. Do I need to complete any additional steps in those tasks?  ☹

Oh, I see. I will review your proposal shortly.

@JosueEchandiaAsto Your proposal does not meet the requirements.

It would be better to keep it, because it is the loading that all the images have when a url is received and it is being loaded.

This spinner while loading the image is better to keep it.
Why should we keep the loader?

@parasharrajat
This is the process:

You are requested to upload an image.
A thumbnail with opacity 0.6 is displayed.
Confirmation of successful upload is received and the list of chat messages is reloaded.
If any message is of type image, with the url we load the image.
A loading is displayed.
Loading the image hides the loading.

In step 3, when reloading the list of chat messages, the thumbnail is lost and what comes from the service is loaded. In this case the white loading is shown. while loading the url that comes from the service. Therefore, it seemed to me that it was better to leave the current post-upload experience.
Now, I can make it save the thumbnail, then validate which message it corresponds to and instead of showing the loading show the saved thumbnail. Is it what you want to do?

Now, I can have it save the thumbnail, then validate which message it corresponds to, and instead of showing the load, show the saved thumbnail. Is that what you want to do?

It would be best to achieve the expected results from this issue. let's see your proposal and we can analyze if it fits well.

@parasharrajat Of course yes, I implement it and send an update of the proposal. Thank you very much.

I think the ideal experience should be

User uploads image
The user sees the thumbnail of the local right away
Note if the user is online, the thumbnail should show with full opacity. The transparency should only appear if the user isn't online imo (cc @chiragsalian)
The image never changes, we don't show a loading image/spinner, to the user uploading the image it should look as if it worked instantly

@JosueEchandiaAsto Am I right in understanding you are considering a proposal for this change?

@MitchExpensify Hello, I have been preparing another proposal but it is complicated to recover the local image when the image post-upload service is called again. At the moment, I am looking at other Issues.

Got it! Thanks for that context.

Price doubled!

Not overdue. Switching back to weekly since we are awaiting proposals.

Doubled!

Proposal
The uploaded image goes through two phases. First, it is shown from the local URL, then, once it’s uploaded, from the server URL. This causes flickering and the loading icon. Flickering is also caused by resizing the image from the default size when re-rendering.
ImageWithSizeCalculation
First, let’s add a new localUrl property so we can distinguish in which phase we are at the image component level. It takes precedence over the url prop.
https://github.com/Expensify/App/blob/773b93f296da193d030a4859b425bb9f74d366f7/src/components/ImageWithSizeCalculation.js#L107
+ const uri = this.props.localUrl || this.props.url;

Remove contain mode, which causes resizing and does not change the appearance of the image because it is fully contained in the component given that its size is calculated from the image itself. Additionally, we set the defaultSource to the same value on the web, otherwise, the RN component flickers. There is an issue related to Avatar flickering where the contributor describes this bug extensively.
https://github.com/Expensify/App/blob/773b93f296da193d030a4859b425bb9f74d366f7/src/components/ImageWithSizeCalculation.js#L120
- source={{uri: this.props.url}}
- resizeMode="contain"
+ source={{uri}}
+ defaultSource={Platform.select({web: {uri}, default: undefined})}

Only show the upload icon if we are not uploading the image
https://github.com/Expensify/App/blob/773b93f296da193d030a4859b425bb9f74d366f7/src/components/ImageWithSizeCalculation.js#L125
- {this.state.isLoading && (
+ {this.state.isLoading && !this.props.localUrl && (

Remove the default values which cause flickering. This change does not affect the image in any way because the size is calculated.
ThumbnailImage
https://github.com/Expensify/App/blob/4b4db2365cd444d5c1f75ab7489f34b4bf60bcc5/src/components/ThumbnailImage.js#L33-L34
- imageWidth: 200,
- imageHeight: 200,
+ imageWidth: undefined,
+ imageHeight: undefined,

Pass the prop through.
https://github.com/Expensify/App/blob/4b4db2365cd444d5c1f75ab7489f34b4bf60bcc5/src/components/ThumbnailImage.js#L103-L106
+ localUrl={this.props.localUrl}

Now we have to make sure that once the image is rendered in Phase 2, it is shown from the local URL instead of the remote URL, which results in no image to show until it’s downloaded. Any subsequent re-render will be fetched automatically from the remote URL.
We do that by temporarily holding the original filename through the phases, then we delete the reference.
The image tag in Phase 2 is generated from HTML received from the server. The uploading process is asynchronous and the original file name is the only way how to correlate both phases.
ImageRenderer
We add a simple object that refreshes when the app loads.
https://github.com/Expensify/App/blob/3439796754d31accfdd4b116afe0eba598278f1b/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js#L9
+ const currentUploads = {};

Here, imagePreviewModalDisabled indicates that we are in Phase 1. If that is the case, normalize the image name because the server does it too. Set localUrl to the current URL. If that is not the case, we are in Phase 2, so fetch the cached local URL, assign it to localUrl , and delete it from the cache. https://github.com/Expensify/App/blob/3439796754d31accfdd4b116afe0eba598278f1b/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js#L50
+ const normFileName = originalFileName.replaceAll(/[\s-._]/g, '');
+ let localUrl = null;
+ if (imagePreviewModalDisabled) {
+     currentUploads[normFileName] = previewSource;
+     localUrl = previewSource;
+ }
+ else if (normFileName in currentUploads) {
+     localUrl = currentUploads[normFileName];
+     delete currentUploads[normFileName]
+ }

Assign the value to the new prop.
https://github.com/Expensify/App/blob/3439796754d31accfdd4b116afe0eba598278f1b/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js#L52
+ localUrl={localUrl}

Pass it through.
https://github.com/Expensify/App/blob/3439796754d31accfdd4b116afe0eba598278f1b/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js#L71
+ localUrl={localUrl}

In Phase 2, the server attaches the original filename to the data-name attribute. In Phase 1, we render the tag ourselves. We have to attach it the same way in order to be resolved to the originalFileName variable used above.
ReportActionItemFragment
https://github.com/Expensify/App/blob/c9eea6c71e68abb088474cecf3f62ac4b870ae08/src/pages/home/report/ReportActionItemFragment.js#L77
- <RenderHTML html={`<comment><img src="${props.attachmentInfo.source}" data-expensify-preview-modal-disabled="true"/></comment>`} />
+ <RenderHTML html={`<comment><img src="${props.attachmentInfo.source}" data-name="${props.attachmentInfo.name}" data-expensify-preview-modal-disabled="true"/></comment>`} />

Remove the default values which cause flickering. This change does not affect the image in any way because the size is calculated.

Would that work for remotely served images?
If the size is calculated from the image it has to first be fetched, what will happen in the meantime (e.g. slow connection)
It might work instantly for local images but will have delay for served content

We can set it conditionally instead. If the image is rendered when uploading (including Phase 2), keep it undefined, otherwise set the original default values.

I'm back from my day off and will be reviewing the proposal shortly…

@marktoman Your proposal seems pretty well reasoned but I'd love to see it in action, especially with a slow/throttled internet connection.

Thanks for the screen captures @marktoman. I like your proposal. If @parasharrajat agrees, then let's go with it!

@parasharrajat Any thoughts to add?

Or Do you think we can preserve the localstate? If we can, this could have been achieved using URLs only.

We cannot because each comment image rendered on the screen creates a new ImageRenderer instance. When uploading, we get two independent instances originating from two places:

When we start uploading, in which case we control the instance (here). This instance is disposed of when the 2. point happens.
When the server sends us the HTML (here). Our uploaded image is now part of the comments with no relation to the uploading event.

Is it the best place to store this?

I believe it is because ImageRenderer is the top-level component in this context and already has the uploading-related logic (code that uses imagePreviewModalDisabled). Splitting this high-level logic and drilling part of it down to the low-level components has no benefit and goes against the separation of concerns principle. We should pass down only the resolved value that is required by the low-level component.

I got it. So what if we keep the same RenderHTML instance and just update the HTML. Will it still create a new instance of ImageRenderer? I guess it will. What do you think?

The event in 2. point happens regardless. If we preserve the RenderHTML instance created in 1. to 2., we still have to somehow decide that what we receive in 2. is the same image as 1. and ignore it, otherwise we end up with two representations of the same image on the screen. (Deciding at this level would require additional parsing, so it is better to take advantage of the regular parsing process and decide at the ImageRenderer level.)

Ok, we can move forward with this implementation for now. A suggestion would be move out the currentuploads logic to another lib and use it inside IMGrenderer.
@marktoman's proposal looks good to me.
cc: @deetergp
:ribbon: :eyes: :ribbon:  C+ reviewed

You can see the image flickering for a split second, but I'm guessing that's a delay from loading image from local storage

Is there a way to prevent the flickering showed in the video for native here? I see it didn't happen on the native video here, was it fixed or is there a different reason this happened on one video but not the other?

The quote is from a different proposal. Both videos reference my proposal, which does not cause flickering and the videos do not flicker.

Awesome, thanks for clarifying

Thanks for the discussion, everyone. @MitchExpensify let's hire @marktoman to move forward with his proposal 👍

Any updates on this?

Not overdue, waiting on a PR. Bumping it down to Weekly.

How is the PR coming along @marktoman ?

How is the PR coming along @marktoman ?

I am experiencing inconsistent behavior on ios. I can push it as a draft PR by Monday, but I'll need more time after that to find a way.

Any update on this one @marktoman?

Let me give it another try this week to finally decide if this problem makes my solution unworkable.

Awesome, thanks @marktoman!

I think this issue has dragged on for a long period of time. It would be really great to push forward on this as soon as we can because it's adversely affecting a core part of the New Expensify experience.
Also can someone confirm if we are going to kill this text as a part of improving the experience? I think it's super old school and unnecessary 🔪

@parasharrajat Do I still have this week to conclude or suddenly not?

@parasharrajat You cannot open it to new proposals after I got confirmed that I have this week to improve the proposal that you have approved. Nobody self-respecting can work like that.

Do I still have this week to conclude or suddenly not?

We'll definitely honor that timeline @marktoman. Lets see if we can come up with a valid solution in the next 3/4 days and go from there

@deetergp Mind assigning @marktoman to the job? Thank you

@parasharrajat Thanks for clarifying. I was hoping you don't mean it right away, but it could be read that way. I'll give it one last try over the weekend and post a conclusion.

Sounds good, thanks @marktoman!

The issue, which affects all name-based proposals, is that if a new file has the same name as a previously uploaded one (e.g., invoice.png), the old file is rendered the same as the newly uploaded one.
My recent effort was to find a way to distinguish them. I am convinced that this is not possible without server changes. The change would be to generate an id for each uploaded file on the client and temporarily store it on the server. The ids can be deleted after x hours because we don't care about old files. The client would use a library like nanoid and send the id alongside the file name. The rest is the same, except we would read the id instead of the file name when distinguishing the newly uploaded file.

The issue, which affects all name-based proposals, is that if a new file has the same name as a previously uploaded one (e.g., invoice.png), the old file is rendered the same as the newly uploaded one.

Ah, interesting. @marcaaron & @tgolen worked on better front-end validation for attachment uploads in this issue. I wonder if they have some insight here?

OK, I am not familiar with this issue, but I took some time to read through the comments. I'm not sure I am 100% there, but I don't think we need any server-side changes for this. That sounds overly complex. When uploading an image, can we just update the image name to have the current timestamp in it like invoice_<timestamp>.png?

but I'm not quite sure what the intention of this is. If all you're trying to do is distinguish between the localURL and the HTML hosted URL, can you search the URL for the s3 domain?

I thought the name should be retained for the user, but if that is not the case, then yes, it can be changed to an id. The intention is to identify a local file and the uploaded remote file as the same. When the user adds a file, it is rendered from a local path, then once uploaded, it comes back from the server and is rendered again from the URL.

I think it'll be OK to add a timestamp to the name. It at least keeps the original name intact so it doesn't look totally foreign to the user, but there is no requirement for keeping the filename 100% the same as the original (that I am aware of).

Basically, the solution is to prevent Phase 2 (ie, the file coming from the server once uploaded) from re-rendering the image.

I think if we have a "local path" for a file that we would just keep using that forever as long as it exists (and only then access the file from the server)

This is the behavior.

the solution is to prevent Phase 2 (ie, the file coming from the server once uploaded) from re-rendering the image

@marktoman can you maybe re-state what the proposal is at a high level without referring to phases? I missed what you are proposing.

@marcaaron This is what happens currently without my solution implemented, which are 2 isolated events:

The user adds an image. The image is rendered from the local URL. At the same time, the image starts uploading to the server.
Once uploaded, the server sends back an IMG element with a remote URL. The client renders the IMG as any other HTML coming from the server.

My solution correlates both events:

When the user adds a file, store both the file name (which can be an id as discussed) and the local path in a temporary map on the client.
Once the server sends the IMG tag, if its name attribute matches a value stored previously (point 1), use the local path instead of the remote URL coming with the IMG tag.

Are there any standard best practices for react native apps when it comes to loading/caching images?
I am concerned about using a temporary in-memory cache for this because it is only going to live as long as the memory of the app (which can be all over the place).

I think the current issue is solely focused on fixing the upload experience and misses an opportunity to address a broader problem
Instead of a temporary map that only servers to cover attachment uploads, we can do the following
Let the Image component handle rendering

We're trying to render a report action containing an image
The Image component first checks if we have a local copy in our cache dir and it instantly loads it (this convers offline usage as well)
Otherwise the Image component has to download (and then cache) content to display it

How the above fixes the Upload experience:

When we upload an attachment we immediately add it to cache
There's no "content flash" because the Image component is not trying to download the image from s3 in order to render it

Having this generic logic added to the Image component should

reduce networking (load from cache instead of making request)
increase image loading speed (loading local content vs s3)
images work offline (as long as they've been seen or uploaded by the current user)
current ticket is also addressed

Yes, thanks! That's much more inline with what I think a true fix would be
for this and sounds like what I would expect to happen.
On Tue, Sep 6, 2022 at 10:57 AM Peter Velkov @.***>
wrote:

I think the current issue is solely focused on fixing the upload
experience and misses an opportunity to address a broader problem
Instead of a temporary map that only servers to cover attachment uploads,
we can do the following
Let the Image component handle rendering

We're trying to render a report action containing an image
The Image component first checks if we have a local copy in our
cache dir and it instantly loads it (this convers offline usage as well)
Otherwise the Image component has to download (and then cache)
content to display it

How the above fixes the Upload experience:

When we upload an attachment we immediately add it to cache
There's no "content flash" because the Image component is not
trying to download the image from s3 in order to render it

Having this generic logic added to the Image component should

reduce networking (load from cache instead of making request)
increase image loading speed (loading local content vs s3)
images work offline (as long as they've been seen or uploaded by the
current user)
current ticket is also addressed

—
Reply to this email directly, view it on GitHub
https://github.com/Expensify/App/issues/9402#issuecomment-1237932342,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAJMABYEFRLRGJYFT4FDG53V44IR3ANCNFSM5YPFFLDA
.
You are receiving this because you were mentioned.Message ID:
@.***>

There's no "content flash" because the Image component is not trying to download the image from s3 in order to render it

There are two challenges:

Loading from the remote URL (the loading icon)
Replacing the local image instance with a new image instance coming from the server (content flash)

Your overview covers 1., but not 2. The 2. point was more time-consuming and it is different for each platform. You can take it from my solution.

Replacing the local image instance with a new image instance coming from the server (content flash)

My understanding of what @kidroca was saying is that this would no longer happen. We shouldn't ever be switching out a locally cached image with an instance coming from the server. If it's fetched from the server, it's because there is no local image.

When the user adds an image, a locally created element is added at the end of the chat. Once the server receives the image content, it sends a new chat HTML that contains the new image. I am not sure how is the locally created element preservable in the stream of new messages.

When the user adds an image, a locally created element is added at the end of the chat. Once the server receives the image content, it sends a new chat HTML that contains the new image. I am not sure how is the locally created element preservable in the stream of new messages.

Yeah, during upload we have an optimistic report action with attachment that's rendered using raw data
After upload it gets replaced by a report action containing html rendered by the HTML Parser
Sample uploaded attachment

I guess the HTML Parser should parse the img tag to our special react-native image component, then it can use the data-name attribute/prop for cache purposes - if a file by that name exists in cache storage use it, otherwise fetch & cache
@parasharrajat

We have to figure out how we want to store the images locally in a cross-platform way.

I think we already figured this out here: https://github.com/Expensify/App/issues/3867

ios/android - Onyx saves paths to local files
web/desktop - files are saved to IndexedDB

they can be feeded as image source

There's been quite a lot of discussion on this and I'm not sure where we landed. Did we decide that backend changes are required and that this is held up on internal changes, or are we able to forge ahead with updates to the front end?

Bump @parasharrajat @marktoman?

Even, I am not sure where we are landed on this. It seems that we discussed not to do https://github.com/Expensify/App/issues/9402#issuecomment-1172569896 in the above comments.
If we are looking to do https://github.com/Expensify/App/issues/9402#issuecomment-1237932342, then it would be good to set a framework to save any attachments instead of just images. Then https://github.com/Expensify/App/issues/6527 comes into the picture before this and thus it should be tracked with https://github.com/Expensify/App/issues/10894

So quick update: We're putting many (pretty much all) Image-related feature request and improvements on a list (see this tracking issue) and we're putting them on HOLD until we design a complete solution for these types of issues. Therefore I'm going to put this issue on HOLD for now.
I love all the ideas so far from @marktoman and @kidroca , rest assured we'll work to compensate everyone fairly who has contributed useful ideas / solutions 👍

Not overdue, just on hold.

Still on hold

@MitchExpensify Snaggin' from you cuz I got the tracking issue assigned to me
[Tracking] [Polish] Image-related feature request and improvements #10894

on hold

This issue is held, Melv.

I'm not following why this issue is on HOLD for a tracking issue - https://github.com/Expensify/App/issues/10894. Maybe that was a mistake and its meant to be on HOLD for something else. Anyone knows?

@Beamanator commented above here. More specifically, it's under the umbrella of image rendering/flashing and this pool of issues is held on implementing better caching.

Now that we are out of planning and have specific issues for the imagine caching, we can update the title to cite #10792 & #12603 specifically.

👋 The native caching PR has merged, so we're down to the one for web/desktop. Updating the title to reflect that we're held only on #12603 at this point.

No change, Melv. Still on hold for the caching PRs

No change, caching PRs still ongoing.

Still on hold

No change, that PR is still ongoing.

Still on hold

Still on hold

Good news, the native side of caching merged today. Web is soon to follow hopefully!

It looks like https://github.com/Expensify/App/issues/12603 is still actively being worked on, so this is still on hold.

@trjExpensify on that linked issue, are we waiting on the upstream fix or have we already forked?

We've forked, and we're waiting on this App PR to test it all. Update is here.

Still on hold.

Same, Melv.

Same, held on cashing that's held on a CORS issue.

Still on hold

Same same.

Still on hold

Still on hold.

Given that caching is held on a CORS issue and that's now held on the server migration, I'm dropping this to monthly until we get closer to a resolution on those two dependencies.

Still on hold.

Still holding

No change from the chain of dependencies here.

No change from the chain of dependencies here.

No change from the chain of dependencies https://github.com/Expensify/App/issues/9402#issuecomment-1531116199.

Getting closer! CORS issue resolved, web image caching PR in development. :)

In light of #32703 being fixed, the new image caching PR is in review.

The image caching PR was reverted again on Friday, so we're back to figuring that out in #12603.

What is the next step here: waiting for #12603 to ship and then what?  Who is doing it?

@kidroca is this still accurate, so once #12603 is out the door there won't actually be any follow-up work necessary here?

@kidroca is this still accurate, so once #12603 is out the door there won't actually be any follow-up work necessary here?

While #12603 moves attachment authentication to headers and promotes better caching it won't fully solve the upload experience process, because we don't have control over native browser cache.
What I'm talking about in comment you refer to is that we can add small alterations and leverage the Cache API (which is different from native browser cache)

Once we upload an attachment we instantly add it to cache
When we try to load remote attachments we first check cache
in case we have cache the image component renders instantly
out of the box result: the image can be displayed while uploading

Potential drawbacks

A similar solution would have to be applied for native iOS/Android
We have to manage cache ourselves - this can be a thin cache layer, or a more complicated one depending on needs, but the least we'll have to monitor quota and cleanup older files

Alternative solution that is simpler

use the local image during uploads
drawback: it only work for the current browser session

if we're offline and the image didn't upload the next time we open a chat we won't see the image

Cool, going to solicit a few opinions to weigh in on that.

Hey @kidroca, I circled this internally and we'd prefer to explore the path that adds a more robust caching solution than simply relying on local storage -- which is viable, but only for the current browser session, after which we could very well lose your attachments.
@roryabraham had some parting thoughts:

The Cache API sounds appropriate for this use case, but I'm pretty unclear on what the native equivalent would be. If it's very much file-driven, maybe the API could be implemented more consistently across platforms if we leverage OPFS on web instead.
Leveraging service workers could be a way to improve performance of our caching layer, such that the caching doesn't happen on the main thread
I think it would be a good idea to design a caching solution holistically with more use-cases in mind (i.e: caching html documents, fonts, and the JS bundle as well, in addition to images). We don't need to include this in the V1 but we should at least think about it.

Is this a project you would be interested in fleshing out and taking the lead on?

Hey @trjExpensify thanks for considering me for this project.
I'm aligned with @roryabraham points and ready to present and explore our options
To ensure I'm on the right track, could you clarify what "fleshing out" entails? Are we looking for a detailed proposal or a preliminary outline of potential approaches?
Looking forward for your guidance and eager to get started.

@kidroca I think a high-level overview of your proposed approach, formatted as a Problem / Solution would be the best way to move this forward. In fact, just aligning on a clear problem statement seems like a great first step.
Then, once you spend some time with the problem and do some initial investigation, you can host a "predesign" in slack, where you present the problem and the potential solutions. Keeping this high-level (and in plain English, no code) is a best practice.
Then, once we've aligned on the best high-level approach, you can produce a detailed implementation plan for the solution we've agreed upon, again using mostly plain English rather than code.
Finally, once we've aligned on the problem and the detailed implementation plan for the solution, we can begin implementation in earnest and review the code.
Does that plan of action make sense?

Problem statement
Predesign high level solutions in #expensify-open-source
Predesign detailed implementation of our preferred solution, either in #expensify-open-source or in this GH issue
Implement

I look forward to seeing what you produce. Don't hesitate to reach out if you get stuck or have questions. Thanks!

Put a larger plan here: https://expensify.slack.com/archives/C01GTK53T8Q/p1708849229702429?thread_ts=1707270017.261459&cid=C01GTK53T8Q

Ok, I think the plan is:

@kidroca HIGH: Add support for inline image Markdown -- This allows us to insert images into the actual HTML itself, which is a precursor to everything.
Maybe @tgolen - Once we have the above, [HOLD ON 37246] HIGH: Combine image attachment with the unsent composed message?
Then we do this issue, which is really evolving into "add large object uploading".

Nice! For (3) do you want to use this issue, or start afresh with a new "clean" one if it's evolving into a larger plan for large object uploading?

3. Then we do this issue, which is really evolving into "add large object uploading".

Let's create a new ticket for large file uploads. The current ticket aims to enhance the upload interface, as shown in the screenshot. We plan to replace the blank rectangle with a grayed-out image preview during uploads. Addressing large uploads is a separate issue and won't fix this visual gap.

Cool, I'll keep this one open then for the attachment thumbnail on upload. :)

@kidroca with #12603 out the way, are we holding this issue on https://github.com/Expensify/App/issues/35977 now or can we move forward with it as part of #3 from here?

@kidroca, now that #12603 is resolved, does #35977 hold us back, or are we clear to proceed with it as outlined in #3 here?

I believe we're not blocked by #35977. Although I'm currently assigned to several tickets, my next focus will be enhancing the upload experience, and I'll be tracking this effort under the current ticket.
For additional context, here's a relevant discussion from Slack.

auth tokens are moved from query params to http headers, this stabilizes the URL and improves caching
I don't think anything is actionable, we need to plan and account for all platforms

I find the current strategy suitable, but it has to be revised a bit, in short it is:

present a slightly transparent thumbnail of the local file during upload (instead of the text "Uploading attachment...")
keep using the local file until close or switch to a different report
the next time we open the report - load attachments from their remote source, this would also cache them
if the attachment is not yet uploaded we still present the local attachment as the optimistic action points to that
The caching layer idea, where uploads are added directly to local cache can be added on top, but this step requires platform specific implementations and managing cache ourselves

Perfect, excited for it! If you do end up needing backend support for this task, give us a shout. 👍

How's it going, @kidroca?

Any update @kidroca?

Hey team, apologies for the radio silence - I've been tied up with some unexpected priorities. I'll be out of office until the end of the week. I'll be focusing on this ticket and making some key enhancements to the upload process.

Nice, look forward to it!

👋 @kidroca what's the plan for this week?

Status Update:
I've been planning the implementation of the upload enhancements. However, I've had to postpone this ticket due to more critical issues that needed my attention.
Current Plan:

Use conditional rendering to display an <Upload> component instead of raw text.
Append local src metadata to the report action to render thumbnails without flashes.
For images, display a thumbnail; for documents or other files, display a generic thumbnail.
Consider directly loading from the backend, allowing a visible transition for user confirmation.

I'll resume work on this ticket this week.

Hey @kidroca. Any progress to report?

Hi, I'm currently tied up with two critical issues that are consuming all of my available time. You can find more details here: https://expensify.slack.com/archives/C05LX9D6E07/p1716402826532549

👋 @kidroca where is this one in the queue now?

I've resumed work on this issue and aim to post a PR by end of week

Status update:
I'm working on this, but haven't posted a PR yet

Status Update
I've successfully displayed the local image during upload, but to prevent a content flash, we need a backend update.

When the user uploads a file, we work with the local file and create an optimistic report action with it.
Later, when the action is swapped by the backend-created action, we lack a reliable way to match the remote image address to our local file.
This mismatch causes the image component to load the remote URL, resulting in a brief flash.

To avoid this flash, we can generate a unique attachment ID on New Dot via NumberUtils.rand64() (similar to how report action IDs are generated) and send it as part of the attachment comment payload (under attachmentInfo). The backend can then use it as a data attribute on the <img> component added to the report action (data-attachment-id). Alternatively, we can send the desired <img> HTML as part of the attachment comment. I've done that, but it seems the backend is currently stripping any extra <img> attributes I've added.
This approach would allow us to resolve the attachment from local data:

For upload cases, this would avoid the content flash.
It would save network bandwidth by resolving user-owned attachments from local data.
It would ensure instant loading for other cases.
If the user deleted the content, it would be loaded from the remote source.

Here's a brief demonstration of the current implementation and the content flash:
https://github.com/Expensify/App/assets/12156624/0edb3a52-7d6b-44d8-a85e-5c9174f8275c

Did we bring this out to Slack yet? Personally, I wouldn't be against splitting this out into two steps. The first being to ship what you've got to display the thumbnail instead of Uploading attachment... which is a good improvement in itself, and the second step to address the content flash. CC: @Expensify/design @JmillsExpensify as well.

Yeah I'm not against splitting it out either. I agree that displaying the thumbnail instead of Uploading attachment... is an improvement. And while the content flash is annoying, it's not the end of the world. So taking care of that in a follow up would be ok with me.

Yup, I agree with that sentiment as well.

Ok, I'll post an update that adds image / video preview during attachment upload
I'm tidying up edge cases and testing across all platforms

👋 How you getting on @kidroca?

Status Update
Hi team,
I'm back from OOO and will be posting a PR for this feature today. I'm experiencing some issues building the Android app, but I'll post the PR regardless and work on resolving the issue later.
I've encountered a code change that introduced a new element <uploading-attachment> (details here: PR 42516), which affects my changes.
I've posted a comment requesting more information on the related issue

https://github.com/Expensify/App/issues/42297#issuecomment-2210595399

I'm having trouble viewing images on local web, I have the same problem on latest main and I'm not sure what the problem is. If I go to new.expensify.com the images do appear
This is the only thing similar I've discovered on slack: https://expensify.slack.com/archives/C01GTK53T8Q/p1719897451272009?thread_ts=1714117518.672049&cid=C01GTK53T8Q
Images just appear as forever loading, not just the ones I send now, but all the past attachments that I have in my chats

Status Update
I was able to verify my PR on all platforms and it's ready for review:

https://github.com/Expensify/App/pull/44889

Status Update
✅ The PR has been reviewed.
Several bugs were discovered during the review, some of which were pre-existing. We have decided to address these in follow-up PRs. I will take responsibility for these follow-ups and report progress as we resolve each item. The issues related to image upload are as follows:

[ ] Bug: Preview closes automatically and chat is redirected to another chat

This will be addressed by leveraging local content when available instead of switching to the remote source which causes the original item to disappear and auto-close the carrousel.

[ ] Bug: No preview while sending a video offline

When offline, attempting to send a video results in a "You appear to be offline" screen when trying to open the carousel.

[ ] ❓Bug: No preview for PDFs until the message is replaced with server response

The backend generates a thumbnail image for document previews. Currently, a generic download link is displayed for documents until they are uploaded and replaced by the backend.

This is how PDF uploads look like. Do we need to take further action here?
https://github.com/user-attachments/assets/7b0f82f0-fcde-4837-a7c3-b4aacb943389

[ ] Bug: Video upload does not display the play button in the middle of the thumbnail

Details: Issue Comment

[ ] Bug: Quick flash or blink when the image transitions from the optimistic version to the actual uploaded version

This will be addressed by leveraging local content when available instead of loading the remote source.

I will post my plan for the next follow-up PR in a new comment and keep this list updated as we check off each item.

I'd like to elaborate on the next steps for enhancing content management by utilizing local data effectively.
Implementing Data Attributes for Efficient Local Content Management
As we've discussed, the primary enhancement I propose is utilizing local content when available. To map remote sources to local content effectively, we need to introduce an attachmentID generated by the client and included in requests. This identifier has been a topic in our recent Slack discussions, particularly in our latest thread which can be found here: Slack Conversation.
The implementation would involve adding a data-attachment-id attribute to HTML elements like images, videos, and anchors. This attribute would act both as an identifier and as a potential final name for the attachment. However, this requires a backend modification to the AddAttachment and AddTextAndAttachment actions, as currently, they do not retain any data attributes from the HTML I'm sending.
My proposal is to include this data-attachment-id on the returned <img>, <video>, and <a/> tags for content uploaded by users. Since my last PR, we've transitioned from sending placeholders like "Uploading image..." to sending actual HTML markup for attachments. The issue now is that any additional html attributes I send are not retained, the backend seems to process replace the html and sends img/video/anchors with the default data attributes and values.
Once implemented, this identifier would allow us to cache local paths to attachments. This means that:

Attachments will render immediately using the local path, preventing the flash that occurs when local sources swap with the remote sources.
When loading a chat, attachments with locally available paths would load instantly, while those without local copies would still fetch from the backend as usual.
This approach will also address the "Preview gets closed" bug, as the source of an attachment during and after upload would consistently be the same local path.
Local paths would be cached inside Onyx during upload. They will be keyed by the unique attachment id and we can store some additional file metadata like dimensions

Regarding other issues, I'll update my list of follow up work. If I find any of these issues easy to fix, I will prioritize them as my next follow-up. Otherwise, my focus will remain on implementing the main enhancement discussed here.
Meanwhile, discussing and setting the backend changes in motion would be beneficial.

Opened a follow-up PR to fix the console error when uploading pdfs

Issue: https://github.com/Expensify/App/issues/46615#top
PR: https://github.com/Expensify/App/pull/47184

The PR is marked as draft as I haven't yet uploaded videos and tested on all platforms

Just removed the payment holds for melv. 👍

The follow-up PR is ready for review, but has an eslint error that I need to fix

https://github.com/Expensify/App/pull/47184

The rest of the changes are blocked as they need the backend update described here: https://github.com/Expensify/App/issues/9402#issuecomment-2266099020

The rest of the changes are blocked as they need the backend update described here: https://github.com/Expensify/App/issues/9402#issuecomment-2266099020

@deetergp are you working on those?

@deetergp are you working on those?

I will look into these later today.

I was not able to get into this this week but will jump in it first thing when I'm back on Tuesday.

Still have not had a chance to look into this deeper. Hoping to do so tomorrow.

In response to this comment calling for back-end changes to allow data-attribute-id, I believe I figured out where we are stripping it from AddAttachment & AddTextAndAttachment and how to add it back. @kidroca do you have a WIP PR that I could use to test these back end changes?

Rebump @kidroca

@deetergp Looks like you like cooking. This link takes to a recipe.

Haha, yes I was cooking. Dish turned out to be pretty tasty too!
This is the comment from @kidroca I had intended meant to link to https://github.com/Expensify/App/issues/9402#issuecomment-2266099020 since Github has decided to bury it.

I'm going to move this to Weekly till we hear back from @kidroca.

Still no news from @kidroca. You still out there?

Still no word from @kidroca. Do we need to open this back up to other contributors?

Do we have a clear picture of what we need done in the front-end PR for that? In the meantime, I've reached out. 👍

Hi team, I apologize for my extended absence and lack of communication. I'm back now and reviewing the current state of the ticket. I'll provide an update within 24 hours, after I've caught up with the latest changes

Status Update
I've synchronized my PR with latest changes from main.

https://github.com/Expensify/App/pull/47184#top

I've tried including data-attachment-id to the markup we send when creating attachments, but re-inspecting the code I've realised this markup is only being used in the optimistic data.

We can generate attachmentID and send it as part of the parameters for API.write (details below).

next: I'll re-test the fix in the PR and add screenshots so it's ready for review.

We could do the following:

When we create the attachment report action (on the client)
We generate an attachmentID via NumberUtils.rand64()
We add a parameter like parameters.attachmentData to the "AddAttachment" API, where we store attachment related information
When the backend creates the action report action and markup it can read the attachment-id from parameters.attachmentData.attachmentID

Or instead of parameters.attachmentData we can introduce, parameters.attachments and move any attachment information like file there. This way when we support multiple attachments they can be listed in parameters.attachments as [{file: ..., attachmentID: ...}, {...}, ...]

My original understanding was that "AddAttachment" API worked with the html markup generated on the client, so the original proposal was to add data-attachment-id to the markup generated here:
https://github.com/Expensify/App/blob/30d508b1e9c2b8598965ffc2e722f830a0a44cfb/src/libs/ReportUtils.ts#L4234-L4248
But inspecting the Network call I see no markup is being sent, or at least the <img> part is being omitted, double checked the code at Report.addActions and it seems that indeed we send only reportCommentText

The issue here is that on the back end, we get the file information from PHP's built-in $_FILES global which has a pre-defined set of parameters. I'm a little uncertain about how we'd get the attachmentID to track with the files 🤔

It's possible we could pass a separate parameters.attachments array then loop through the $_FILES array and try to associate the  additional metadata passed in the attachments array with it.

Just so I am crystal clear on what we need on the back end here: The end result we are looking for here is that that we will add data-attachment-id=<the ID passed from NewDot> to the image tag in the HTML we save to the message field on the ADDCOMMENT report action. Is that correct?

Just so I am crystal clear on what we need on the back end here: The end result we are looking for here is that that we will add data-attachment-id=<the ID passed from NewDot> to the image tag in the HTML we save to the message field on the ADDCOMMENT report action. Is that correct?

Yes

It's possible we could pass a separate parameters.attachments array then loop through the $_FILES array and try to associate the additional metadata passed in the attachments array with it.

Yes, both these lists should have the same order when sent
I originally thought that we send file metadata as JSON, but it seems we only send binary data for the file, so any metadata like the attachment id should be added under a separate key
Since we currently support only a single attachment let's add a key like attachmentMeta or attachmentData to the payload for AddAttachment and AddTextAndAttachment
Or if we like to keep it super simple we can just add the key attachmentID to these actions

Just so I am crystal clear on what we need on the back end here: The end result we are looking for here is that that we will add data-attachment-id=<the ID passed from NewDot> to the image tag in the HTML we save to the message field on the ADDCOMMENT report action. Is that correct?

Actually we need to add this to all attachments not just <img>, but <video> and <a> for attachments as well

Or if we like to keep it super simple we can just add the key attachmentID to these actions

Let's keep it simple for now. I have a back end PR updated and out for review.

The PR to add this is now on staging.

Great stuff, that's now on prod! @kidroca can you give us an idea of when you can progress the front-end PR? Thanks!

Bump @kidroca

Randomly ended up here, bumped to Daily and made @kidroca the owner to get 👀.  I'll ping him too

I removed @kidroca & @marktoman & reapplied the Help Wanted label


[ACC-136] Update auditing metadata attributes and tests

Remove tests about checking the If-Unmodified-Since during content creation from AuditMetadataTest, because the rest-controller from spring-content uses at the last modified date of the file (and not of the entity) when checking the header. Since there is no content yet when you upload a file these test are not supposed to work.
Update database column names of audit metadata attributes to reflect what is generated
Update setter of audit metadata attributes to reflect what is generated

Can you split off the removal of the tests? It's not directly related to the audit metadata feature.
Additionally, I would like some tests that verify the correct behavior of the content endpoints. Since it's not related to audit metadata, maybe in a different class.


Update operations_guide.rst

orderer1-org0: registration name and enrollment name are not the same.
admin-org0: registration name/password and enrollment name/password are not the same.

@sungjinwang  - you need to sign-off for your commit:   https://hyperledger-fabric.readthedocs.io/en/release-1.4/CONTRIBUTING.html#legal-stuff


Neighbor Solicitation Response Fix

Description
IPv6 Neighbor Solicitation Responses could go out the wrong interface. Fixed by using pxNetworkBuffer->pxEndPoint instead of searching for and end-point.
I allowed myself the liberty of cleaning up a bit and also enforcing that pxNetworkBuffer, pxNetworkBuffer->pxInterface, and pxNetworkBuffer->pxEndPoint are non-NULL for ALL received packets.
@shubnil , @htibosch, @HTRamsey,  Please let me know what you think
Related Issue
#1093
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Assuming all buffers, interfaces, and endpoints downstream from ipCONSIDER_FRAME_FOR_PROCESSING massively simplifies the cyclomatic complexity of everything else and makes it more determinant, which I am a big fan of, but I don't know how the others will feel. I do believe there is a lot of redundant/wasteful checking that makes test coverage more difficult though.

Please check out my latest commit and tell me if it's a step in the right or wrong direction.

I squashed all the changes and force-pushed this to make it easier to look at.
@tony-josi-aws, I'm sorry but I'm out of time. If you think anything else needs to change, please feel free to make the changes.

/bot run formatting


async support

Support for async injection. Nice to have, but low priority currently.

Added in the v0.11.0
from antidote import inject

@inject
async def f():
    pass


.Net: The API deployment for this resource does not exist

Describe the bug
A clear and concise description of what the bug is.
Error: Microsoft.SemanticKernel.HttpOperationException: The API deployment for this resource does not exist. If you created the deployment within the last 5 minutes, please wait a moment and try again.
Status: 404 (Not Found)
ErrorCode: DeploymentNotFound

Content:
{"error":{"code":"DeploymentNotFound", "message":"The API deployment for this resource does not exist. If you created the deployment within the last 5 minutes, please wait a moment and try again."}}

Headers:
apim-request-id: REDACTED
x-ms-client-request-id: 843657a2-f43f-49da-a1f5-6ecc4206e71f
Strict-Transport-Security: REDACTED
X-Content-Type-Options: REDACTED
x-ms-region: REDACTED
Date: Wed, 03 Jan 2024 13:28:55 GMT
Content-Length: 198
Content-Type: application/json

---> Azure.RequestFailedException: The API deployment for this resource does not exist. If you created the deployment within the last 5 minutes, please wait a moment and try again.
Status: 404 (Not Found)
ErrorCode: DeploymentNotFound

Content:
{"error":{"code":"DeploymentNotFound", "message":"The API deployment for this resource does not exist. If you created the deployment within the last 5 minutes, please wait a moment and try again."}}

Headers:
apim-request-id: REDACTED
x-ms-client-request-id: 843657a2-f43f-49da-a1f5-6ecc4206e71f
Strict-Transport-Security: REDACTED
X-Content-Type-Options: REDACTED
x-ms-region: REDACTED
Date: Wed, 03 Jan 2024 13:28:55 GMT
Content-Length: 198
Content-Type: application/json

at Azure.Core.HttpPipelineExtensions.ProcessMessageAsync(HttpPipeline pipeline, HttpMessage message, RequestContext requestContext, CancellationToken cancellationToken)
at Azure.AI.OpenAI.OpenAIClient.GetImageGenerationsAsync(ImageGenerationOptions imageGenerationOptions, CancellationToken cancellationToken)
at Microsoft.SemanticKernel.Connectors.OpenAI.AzureOpenAITextToImageService.GenerateImageAsync(String description, Int32 width, Int32 height, Kernel kernel, CancellationToken cancellationToken)
--- End of inner exception stack trace ---
at Microsoft.SemanticKernel.Connectors.OpenAI.AzureOpenAITextToImageService.GenerateImageAsync(String description, Int32 width, Int32 height, Kernel kernel, CancellationToken cancellationToken)
at Submission#10.<<Initialize>>d__0.MoveNext()
--- End of stack trace from previous location ---
at Microsoft.CodeAnalysis.Scripting.ScriptExecutionState.RunSubmissionsAsync[TResult](ImmutableArray`1 precedingExecutors, Func`2 currentExecutor, StrongBox`1 exceptionHolderOpt, Func`2 catchExceptionOpt, CancellationToken cancellationToken)

To Reproduce
Steps to reproduce the behavior:

Go to '07-DALL-E-3' on notebooks folder
Run  Generate a (random) image with DALL-E 3
See error

Expected behavior
A clear and concise description of what you expected to happen.
A cure bear image is ecpected.
Screenshots
If applicable, add screenshots to help explain your problem.
kernel is created succefuly

Generation gets error

Platform

OS: Windows
IDE: VS Code
Language: C#
Source: main branch of repository

Additional context
Add any other context about the problem here.

When I follow below doc, it runs well
https://learn.microsoft.com/en-us/azure/ai-services/openai/dall-e-quickstart?tabs=dalle3%2Ccommand-line&pivots=programming-language-csharp
using System;
using System.IO;
using System.Threading.Tasks;
using Azure.AI.OpenAI;

namespace Azure.AI.OpenAI.Tests.Samples
{
    public partial class GenerateImages
    {
        // add an async Main method:
        public static async Task Main(string[] args)
        {
            string endpoint = GetEnvironmentVariable("AZURE_OPENAI_ENDPOINT");
            string key = GetEnvironmentVariable("AZURE_OPENAI_KEY");

            OpenAIClient client = new(new Uri(endpoint), new AzureKeyCredential(key));

            Response<ImageGenerations> imageGenerations = await client.GetImageGenerationsAsync(
                new ImageGenerationOptions()
                {
                    Prompt = "a happy monkey eating a banana, in watercolor",
                    Size = ImageSize.Size256x256,
                });

            // Image Generations responses provide URLs you can use to retrieve requested images
            Uri imageUri = imageGenerations.Value.Data[0].Url;

            // Print the image URI to console:
            Console.WriteLine(imageUri);
        }
    }
}

ImageGenerationOptions class taking not nullable DeploymentName and passing 'dall-e-3'  or any other model throws above exception.

Confirmed that Azure.AI.OpenAI --version 1.0.0-beta.7 works but from beta.8 ~ 12 throws exception


Scraper Tasks Button Actions not working , resume and stop.

Run is working fine , look for last commit for changes in template structure

Resume , adds scraper to queue.
Stop not functioning atm


standard level unit test bug?

HI @s-good,
I've been picking through the EN suite over on no-global lately, and I noticed that this line in the unit tests looks off; the unit test claims to be looking for a temperature excursion, which it creates in the fake profile a few lines above, but the line linked here shows no flags for any level in the profile, which seems wrong (shouldn't the last one be flagged as a temperature excursion) - which is especially disturbing since the unit test passes! Either there's something more subtle than I currently appreciate happening here, or there's something wrong with this unit test which could possibly be masking a pathology with the qc test itself.

This looks like it reproduces a test that is run for the EN background check. When run on reported levels one of the levels gets rejected, but on standard levels it doesn't. Possibly it is not the most useful example, but there are some other tests further down where tests are run on real profiles which are more useful.

Ah, so the standard level test isn't actually supposed to catch that temperature excursion - ok, that makes sense then!


add datanode query client

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@b49f4b9). Click here to learn what that means.
The diff coverage is 74.28%.

@@            Coverage Diff            @@
##             master     #202   +/-   ##
=========================================
  Coverage          ?   74.36%
=========================================
  Files             ?      148
  Lines             ?    19872
  Branches          ?        0
=========================================
  Hits              ?    14778
  Misses            ?     3975
  Partials          ?     1119

Impacted Files
Coverage Δ

datanode/client/query_client.go
74.28% <74.28%> (ø)

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 b49f4b9...8a1f8da. Read the comment docs.


KeyError: 'action'

Hi, I tried launching r2d1 on atari using atari_r2d1_async_alt and I get this error:
call string:
 taskset -c 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22 /home/mila/n/nekoeiha/.conda/envs/rlpyt/bin/python /home/mila/n/nekoeiha/MILA/rlpyt/rlpyt/experiments/scripts/atari/dqn/launch/pabti/../../train/atari_r2d1_async_alt.py 0slt_24cpu_4gpu_0hto_1ass_2sgr_1alt /home/mila/n/nekoeiha/MILA/rlpyt/data/local/20200824/161821/atari_r2d1_async_alt/gravitar 0 async_alt_pabti
Unable to import tensorboard SummaryWriter, proceeding without.
using seed 131
Traceback (most recent call last):
  File "/home/mila/n/nekoeiha/MILA/rlpyt/rlpyt/experiments/scripts/atari/dqn/launch/pabti/../../train/atari_r2d1_async_alt.py", line 48, in <module>
    build_and_train(*sys.argv[1:])
  File "/home/mila/n/nekoeiha/MILA/rlpyt/rlpyt/experiments/scripts/atari/dqn/launch/pabti/../../train/atari_r2d1_async_alt.py", line 44, in build_and_train
    runner.train()
  File "/home/mila/n/nekoeiha/MILA/rlpyt/rlpyt/runners/async_rl.py", line 87, in train
    throttle_itr, delta_throttle_itr = self.startup()
  File "/home/mila/n/nekoeiha/MILA/rlpyt/rlpyt/runners/async_rl.py", line 147, in startup
    seed=self.seed,
  File "/home/mila/n/nekoeiha/MILA/rlpyt/rlpyt/samplers/async_/alternating_sampler.py", line 24, in async_initialize
    return super().async_initialize(agent, *args, **kwargs)
  File "/home/mila/n/nekoeiha/MILA/rlpyt/rlpyt/samplers/async_/base.py", line 34, in async_initialize
    subprocess=True)  # Would like subprocess=True, but might hang?
  File "/home/mila/n/nekoeiha/MILA/rlpyt/rlpyt/samplers/buffer.py", line 29, in build_samples_buffer
    all_action = buffer_from_example(examples["action"], (T + 1, B), agent_shared)
  File "<string>", line 2, in __getitem__
  File "/home/mila/n/nekoeiha/.conda/envs/rlpyt/lib/python3.7/multiprocessing/managers.py", line 834, in _callmethod
    raise convert_to_error(kind, result)
KeyError: 'action'

I'll be grateful if you could help me figure it out.

Yes, sorry this is an uninformative error I sometimes run into.  It usually means there is something wrong inside the subprocess which is generating the example action, observation, etc...  One thing to try to get more information would be to set subprocess=False here:
  File "/home/mila/n/nekoeiha/MILA/rlpyt/rlpyt/samplers/async_/base.py", line 34, in async_initialize
    subprocess=True)  # Would like subprocess=True, but might hang?

but it might still be hard to follow since the async sampler opens in its own subprocess.
Let me know if that helps?

I am stuck with this error @astooke ! Any suggestions ?

Yes, sorry this is an uninformative error I sometimes run into. It usually means there is something wrong inside the subprocess which is generating the example action, observation, etc... One thing to try to get more information would be to set subprocess=False here:
  File "/home/mila/n/nekoeiha/MILA/rlpyt/rlpyt/samplers/async_/base.py", line 34, in async_initialize
    subprocess=True)  # Would like subprocess=True, but might hang?

but it might still be hard to follow since the async sampler opens in its own subprocess.
Let me know if that helps?

I encountered a similar error. When I modify 'subprocess=False', it shows as follows:

This should be the early termination of the subprocess which is running the function get_example_outputs during sampler initialization. I'm not sure why the subprocess must be True during the initialization.
But, every time KeyError: 'action' happens when the interaction between agent (at CPU mode) and the environment goes wrong and stops the subprocess. It is either the env instance encountered any error or the agent did not support non-batchwise agent input in agent.step() implementation.
:)


chau hugo + docker, hello milpa help docs --server

Docker and hugo are great but I really wanted to have local http docs available without docker so this PR adds milpa help docs --server to do just that.
It's not as fancy as Hugo and it does add to the binary size but hopefully that difference is meaningless given gzip (both on the released packages and docker images containing milpa, which is all I care about).
breaking changes
I mean, this is still an alpha release, so not breaking in the sense of semver, but more like wildly inconsistent changes, with regards to the previous release:

milpa itself shell install-autocomplete renamed to milpa itself install-autocomplete
milpa itself shell init is gone. I never really used it.
milpa itself docs create is also gone for the same reasons.
milpa itself docs html is gone. This command was used to both render html and use hugo as a dev server.


Fix beacon time interval to match requirement

We require the beacon time to be less than ten minutes, but in fact in code we perform a comparison with thirty minutes. This fix introduces consistency.

Hey thank you for the patch! This looks good. I'll have some more time during the upcoming holiday season to integrate this into the next beta release.

@cceremuga, great, I'll try to help occasionally.


Check subject UTF8 validity just once for String#gsub, #scan, #split

As of #13313, regex matching uses neither NO_UTF_CHECK nor MATCH_INVALID_UTF, which means the subject string is checked for UTF-8 validity once per match. String#gsub, #scan, and #split are special in that the regex doesn't change and is expected to be matched many times, so for these methods we should only need to do this check exactly once; if the first match does succeed, we know that the subject string is validly encoded and we can apply NO_UTF_CHECK in subsequent matches, regardless of whether other optimizations are in place.
Benchmarks:
require "benchmark"

class String
  def scan_new(pattern : Regex, &) : self
    # this PR's implementation
  end
end

data = "a" * 10000
count = 0

Benchmark.ips do |b|
  b.report("old") do
    count = 0
    data.scan(/a/) { count += 1 }
  end

  b.report("new") do
    count = 0
    data.scan_new(/a/) { count += 1 }
  end
end

old  72.43  ( 13.81ms) (± 0.66%)  313kB/op  38.50× slower
new   2.79k (358.60µs) (± 0.94%)  313kB/op        fastest

If #13353 is merged then options |= :no_utf_check would apply to the parameters of the respective methods themselves.

Another candidate is StringScanner whose subject string is supplied in the constructor and also never changes afterwards.


Spring Kafka doesn't returns null for Tombstone records during batch processing

@Payload(required = false) is a great thing during consuming of records that might be eventually Tombstone. But it does not work in a batch processing. Here is an example that does throw ClassCastException once Tombstone record received:
@KafkaListener(topics = ["\${kafka.topic.categories}"])
fun receive(
   @Header(KafkaHeaders.RECEIVED_MESSAGE_KEY) List<String> keys,
   @Payload(required = false) categories: List<Category>
) {
    ...
}

This is especially inconvenient with records represented by JSON, as you have to write additional workarounds to achieve the required functionality. Would be great to avoid any workarounds inside a business code by having support of @Payload as part of a batch processing inside of Spring FW.

@garyrussell is it possible to build a snapshot into spring repo pls, so I can use it in my project?

2.2.5.BUILD-SNAPSHOT containing this is now in the https://repo.spring.io/snapshots repo.


[PT Run] Action key detection has false-positives

Microsoft PowerToys version
0.1.0 (pre 0.53.0)
Running as admin

[ ] Yes

Area(s) with issue?
PowerToys Run
Steps to reproduce
Open PT Run and type ?? as the keyword of Websearch.
✔ Expected Behavior
Only default result of websearch is shown.
❌ Actual Behavior
Web search result and file results are shown until I type a character to search for.

Other Software
No response
Additional notes

File search plugin has no action key defined.
File search si set to global.
PowerToysReport_2021-12-24-10-05-01.zip

Since the Windows Search plugin is using ? as activation command, and the web search plugin is using the ?? command, typing ?? seems triggering both ? and ??

Since the Windows Search plugin is using ? as activation command, and the web search plugin is using the ?? command, typing ?? seems triggering both ? and ??

Yes and no. Normally it should work as expected. We have // for uri as default keyword. I think the same happened there but there are no files for /.
But as I mentioned I didn't set a default keyword for search. So ? can't trigger the files search if PT Run works as expected.

@cyberrex5 , @crutkas , @jaimecbernardo
Should we switch the default activation phrase for web search until the bug is fixed?

https://github.com/microsoft/PowerToys/blob/main/src/modules/launcher/PowerLauncher/Plugin/QueryBuilder.cs#L20-L30
Might be the cause. Maybe trimming is incorrect.

I will look into it this weekend. Please assign it to me.

Added Prio-0 label because we need this fix in version 0.53.0. Without this fix the action phrase for web-search plugin doesn't work as expected.

I suggest to compare the action key with the same number of first chars from query. (Example: Two char action key compared with first two chars from query.)

This will not work, for example: if we have string query = "?? foo bar" and we want to check windows search's action keyword (which is ?), query[0] == '?' would still return true.
one way to fix this would be to loop over every action keyword and check if another plugin's action keyword starts with it, and vice versa. Then we'd remove the plugin with the shorter one.
I can't think of a simpler solution

I run into this a few minutes ago.
We could save us the number of chars in the keyword if < than on last plugin in a var. Then we get the number chars in the longest keyword that match. based on this we then can eliminate all plugins that didn't match. What do you think?

I don't understand.
so we have the number of chars in the longest keyword that matches, and the shortest. What do we do with them?

I don't understand. so we have the number of chars in the longest keyword that matches, and the shortest. What do we do with them?

only the longest. then we remove all pair wich shorter actionkeywords from generated pair-ob´ject

@cyberrex5
Our solution from the PR doesn't work. It bloks some results we like to see in special cases.
Query: ?abc
Keyword of plugins: ?, ??, !
Expected* results from plugins with: ?, ??
Seen results**: ??
*) Based on existing unit tests.
**) Because we check against the longest keyword.
Do you think it's okay if queries with ? as query and the same char as first char on the search term (query without action char) aren't possible? Then we can simply check if the query starts with pluginActionKey + pluginActionKey and don't need any extra loops.

I tried that query and my seen results were from plugins with ? not ??. We check against the longest keyword of the plugins that matched, so i have no idea how that even happened with you.
Also expected results should be only from ?. (see my PR on your branch)
For example here i set the windows settings plugin's keyword to ???, and the uri plugin's' to ?? (same as web search). And the results are as expected:

@cyberrex5
I made a typo on seen results. 😅 It's impossible to ?? results on typing ?abc.
But as I saw in the test it is expected that all plugin that have the typed action key as part of it's configured one are shown. So ? should show ? and ??. Or should we change this behavior completely?

But as I saw in the tests it is expected that all plugins that have the typed action key as prefix of it's configured action key are shown. So ? should show ? and ??. Or should we change this behavior?

I think it makes sense to change it

But as I saw in the tests it is expected that all plugins that have the typed action key as prefix of it's configured action key are shown. So ? should show ? and ??. Or should we change this behavior?

I think it makes sense to change it.

Let's wait on @crutkas' decision.

If the expected result was according to the (current) unit test then the bug in this issue is not a bug, but intentional.

No. The issue here is because ?? shows plugins with ?.

But as I saw in the tests it is expected that all plugins that have the typed action key as prefix of it's configured action key are shown. So ? should show ? and ??. Or should we change this behavior?

I think it makes sense to change it.
If the expected result was according to the (current) unit test then the bug in this issue is not a bug, but intentional.

Hmmm. You are correct. I misunderstood it because I looked on the test title only.

@stefansjfw
Can you please update label and project. Thank you.

@stefansjfw Can you please update label and project. Thank you.

done

The was fixed in in 0.53.  Head over to https://aka.ms/installpowertoys to try it out


_block.LZ4BlockError: Decompression failed (Error code: 30236)

Hi,
I'm trying to extract images from UnityFS bundles but unfortunately it fails
Header is
00000000  55 6e 69 74 79 46 53 00  00 00 00 07 35 2e 78 2e  |UnityFS.....5.x.|
00000010  78 00 32 30 32 31 2e 32  2e 30 62 39 00 00 00 00  |x.2021.2.0b9....|
00000020  00 03 69 cb dd 00 00 12  5f 00 00 12 5f 00 00 00  |..i....._..._...|
00000030  c0 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|

$ unityextract --images -o /tmp/ __data
Traceback (most recent call last):
  File "/usr/bin/unityextract", line 159, in <module>
    main()
  File "/usr/bin/unityextract", line 155, in main
    exit(app.run())
  File "/usr/bin/unityextract", line 54, in run
    bundle = unitypack.load(f)
  File "/usr/lib/python3.9/site-packages/unitypack/__init__.py", line 12, in load
    return env.load(file)
  File "/usr/lib/python3.9/site-packages/unitypack/environment.py", line 28, in load
    ret.load(file)
  File "/usr/lib/python3.9/site-packages/unitypack/assetbundle.py", line 49, in load
    self.load_unityfs(buf)
  File "/usr/lib/python3.9/site-packages/unitypack/assetbundle.py", line 128, in load_unityfs
    storage = ArchiveBlockStorage(blocks, buf)
  File "/usr/lib/python3.9/site-packages/unitypack/assetbundle.py", line 197, in __init__
    self._seek(0)
  File "/usr/lib/python3.9/site-packages/unitypack/assetbundle.py", line 230, in _seek
    self.seek_to_block(new_cursor)
  File "/usr/lib/python3.9/site-packages/unitypack/assetbundle.py", line 256, in seek_to_block
    self.current_stream = self.current_block.decompress(buf)
  File "/usr/lib/python3.9/site-packages/unitypack/assetbundle.py", line 181, in decompress
    res = lz4_decompress(buf.read(self.compressed_size), self.uncompressed_size)
  File "/usr/lib/python3.9/site-packages/unitypack/utils.py", line 11, in lz4_decompress
    return decompress(data, size)
_block.LZ4BlockError: Decompression failed: corrupt input or insufficient space in destination buffer. Error code: 30236

Any ideas?

I'm also having the issue
_block.LZ4BlockError: Decompression failed: corrupt input or insufficient space in destination buffer. Error code: 16
ever since hearthstone update to 21.6 where they updated the unity engine. Does anyone have a solution

I switched to UnityPy. I had to update my code, but the project is actively maintained and has an active Discord community, so it felt like a worthy investment.

diff --git a/unitypack/assetbundle.py b/unitypack/assetbundle.py
index 4c5a77c..8c6f7a1 100644
--- a/unitypack/assetbundle.py
+++ b/unitypack/assetbundle.py
@@ -98,6 +98,8 @@ class AssetBundle:
                self.ciblock_size = buf.read_uint()
                self.uiblock_size = buf.read_uint()
                flags = buf.read_uint()
+               if self.format_version >= 7:
+                       buf.seek((buf.tell() + 15) // 16 * 16)
                compression = CompressionType(flags & 0x3F)
                eof_metadata = flags & 0x80
                if eof_metadata:

maybe you can try to use this fork: https://github.com/CancerGary/UnityPack/tree/fix/2020.1


Actions improvements!

But we cant remove travis, because actions fails on prs

i didn't remove it so

well also the issue is the access to the server to copy it over


Xcode is not required for all actions

Checklist

[x] I've run bundle exec rspec from the root directory to see all new and existing tests pass
[x] I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
[x] I've read the Contribution Guidelines
[x] I've updated the documentation if necessary.

Motivation and Context

We use "sigh" by itself; it does not require Xcode.  A new version of fastlane was failing with the following, which was annoying since this command does not require Xcode.
sh: /Library/Developer/CommandLineTools//usr/bin/xcodebuild: No such file or directory
ERROR [2017-12-13 12:22:40.21]: undefined method `split' for nil:NilClass
sh: /Library/Developer/CommandLineTools//usr/bin/xcodebuild: No such file or directory
ERROR [2017-12-13 12:22:40.24]: undefined method `split' for nil:NilClass
/Users/jenkins/.fastlane/bin/bundle/lib/ruby/gems/2.2.0/gems/fastlane-2.69.2/fastlane_core/lib/fastlane_core/ui/interface.rb:131:in `user_error!': [!] Error detecting currently used Xcode installation, please ensure that you have Xcode installed and set it using `sudo xcode-select -s [path]` (FastlaneCore::Interface::FastlaneError)

Description

Return nil as the xcode_version if usr/bin/xcodebuild does not exist at the xcode-select path

I signed the CLA and corrected my commit to use the same email address.


Increment version for identity releases

Increment package version after release of Azure.Identity

API change check
API changes are not detected in this pull request.


Is it possible to change the ghost image when dragging of the element occurs

There seams to be a ghost image on drag which can't be changed is it possible to change this ghost image to an html element?

Can you please provide a test case (e.g. js fiddle) and tell me the browser, version and OS?
Currently the ghost can not be change, however it should be an easy fix. All you need to do is to allow the user to overwrite _makeGhost via the options passed to the sortable.
A PR would be very welcome.

@lukasoppermann Is this still open?
I see those lines in html5sortable.ts:347:
      // add transparent clone or other ghost to cursor
      setDragImage(e, dragItem, options.customDragImage)

It looks like it was already implemented to me. If not, would love to take it if you can clear up what needs to be done.

@ZiadAliKhalifa I think it is possible if you create a custom function and assign it to options.customDragImage.
To solve this I think it would be good if you could test it e.g. on the docs html document or jsfiddle and document how to do it in the readme afterwards. If you could send a PR with this, that would be very helpful. 👍

@lukasoppermann I would love to work on this issue; please assign it to me 😄

@lukasoppermann
This issue seems to be already fixed. We pass a function to the customDragImage option to accomplish this. One thing we can do is simplify the process. Instead of providing a function, we can pass the query string, such as .drag-image, to the customDragImage option. We can use the query string to get the element and create a function.
sortable('.sortable', {
  customDragImage: '.drag-image',
});

@lukasoppermann
This issue seems to be already fixed. We pass a function to the customDragImage option to accomplish this. One thing we can do is simplify the process. Instead of providing a function, we can pass the query string, such as .drag-image, to the customDragImage option. We can use the query string to get the element and create a function.
sortable('.sortable', {
  customDragImage: '.drag-image',
});

Please give your opinion on this, and I'll start working on this.

Hey, I think we could allow users to pass either a function or a selector string.
If they pass a selector string we create the function for them, if they add a function we run the function (people may want to do something more crazy with it.)

@lukasoppermann Sounds good. I'll make a PR soon 👍

@lukasoppermann
Has this issue been addressed yet?
If you haven't already, I'd like to take care of it.

This issue has been resolved and will be closed.


ArgoCD should only be installed in one cluster.

ArgoCD should only be installed in one cluster, as having one instance per cluster is not the right way to configure it (different installs, users per cluster, etc). Instead, a central ArgoCD installation with proper permissions (using projects, etc), is a much better example of how real-world ArgoCD installations are.
We don't have to go the project and permissions route, but we should at least install ArgoCD in only one cluster and update the existing example applications to use the cluster generator to get them installed in all the registered clusters, preserving the current behaviour.

count on me for that one, when I hopefully will get cycles to work on it... I wish my December would be different...

There are challenges with leveraging the applicationset approach across the clouds (i.e. authentication handling, as the constructs are built on the point of "role" assumption). Few references how to:

[ ] https://github.com/argoproj/argo-cd/issues/11207
[ ] https://github.com/argoproj/argo-cd/pull/9190
[ ] https://github.com/argoproj/argo-cd/issues/9460

Basically or we should static auth credentials (which is impossible) or keep the deployment within the common cloud (which is not a common demo example), i.e. we will hit the challenges when k8s where ArgoCD will be installed... needs to call out another cloud fleet.
Talking fluxcd
https://fluxcd.io/flux/get-started/#multi-cluster-setup
To use Flux to manage more than one cluster or promote deployments from staging to production, take a look at the two approaches in the repositories listed below.

https://github.com/fluxcd/flux2-kustomize-helm-example
https://github.com/fluxcd/flux2-multi-tenancy
Interesting enough, however not necessary demo centric at this stage is fleet https://fleet.rancher.io/concepts - I will dig deeper on that one...
p.s. as meanwhile as much as I want it feels like argocd or fluxcd cannot be yet deployed in a centralized fashion... which leads to the point of doing wrappers around which apps do go to which clusters... https://github.com/tetrateio/tetrate-service-bridge-sandbox/issues/177#issuecomment-1359357097

https://openunison.github.io


Npm start gets stuck at Starting Packager...

Description
npm start gets stuck on Starting packager
Expected Behavior
It should give the QR code on the terminal
Observed Behavior
A few days back it was working fine. Then it worked after I deleted all node_modules file and installed again. But now it doesn't work at all.
If there's an error message, please paste the full terminal output and error message in this code block:
Error text goes here!

Environment
Please run these commands in the project folder and fill in their results:

npm ls react-native-scripts<EMAIL_ADDRESS>npm ls react-native<EMAIL_ADDRESS>npm ls expo<EMAIL_ADDRESS>node -v:v10.0.0
npm -v: 6.0.0
yarn --version: 1.7.0
watchman version: 4.7.0

Also specify:

Operating system: Ubuntu 16.04

I have the same, on;
Environment
Please run these commands in the project folder and fill in their results:

npm ls react-native-scripts<EMAIL_ADDRESS>npm ls react-native<EMAIL_ADDRESS>npm ls expo<EMAIL_ADDRESS>node -v: v10.3.0
npm -v: 6.1.0
yarn --version: n.a.
watchman version: 4.9.0

Also specify:

Operating system: macOS High Sierra 10.13.4

For me the following fixed it;
Uninstalling create-react-native-app & reinstalling it again.

Same issue for me, using npm.
Worth noting that I ran create-react-native-app to create a new project and npm run start works as expected.
Environment
Please run these commands in the project folder and fill in their results:
npm ls react-native-scripts<EMAIL_ADDRESS>npm ls react-native<EMAIL_ADDRESS>npm ls expo<EMAIL_ADDRESS>node -v: v8.11.3
npm -v: 5.6.0
yarn --version: 1.7.0
watchman version: n/a
Also specify:
Operating system: Windows 10

I got the issue.
The issue was in the

/proc/sys/fs/inotify/max_user_watches = 8192
/proc/sys/fs/inotify/max_queued_events = 16384

Basically, max_user_watches impacts how many dirs you can watch across all watched roots and max_queued_events impacts how likely it is that your system will experience a notification overflow. So while building the native app, these limits used to exceed because of which build process got stuck.
Just increasing these limits makes it work.
To increase the limit use:

echo fs.inotify.max_user_watches=524288 | sudo tee -a /etc/sysctl.conf

@esamelson Yes, no luck...


Running Selenoid in restricted docker environment

Hello,
I'm trying to run the selenoid tests in the Bitbucket Pipelines, however, the docker usage in there is quite restricted.
Firstly I faced a problem with the privileged mode.
2020/08/06 14:26:32 [0] [SERVICE_STARTUP_FAILED] [create container: Error response from daemon: authorization denied by plugin pipelines: --privileged=true is not allowed]
Then, after adding the -disable-privileged I'm having a problem with the added capabilities.
2020/08/06 14:29:31 [0] [SERVICE_STARTUP_FAILED] [create container: Error response from daemon: authorization denied by plugin pipelines: --cap-add is not allowed]
Doing some research, I found that people are saying that it should be possible to run Chrome without cap=SYS_ADMIN or privileged mode while using --no-sandbox: https://github.com/Zenika/alpine-chrome#-with---no-sandbox
Can did you try such a setup? Would that work?
I checked the Selenoid code and looks like currently there is no way to run the container without cap=SYS_ADMIN or privileged mode. If that would work maybe that makes sense to add it?
I guess this is related to #934 #251
Thanks.

@Trane9991 Selenoid has the following flag for that case:
-disable-privileged
    Whether to disable privileged container mode

This is described in docs: https://aerokube.com/selenoid/latest/#_selenoid_cli_flags

In the issue description I mentioned that I tried that flag.

@Trane9991 currently using CAP_SYS_ADMIN is hard-coded.

I know, I've seen that in the code.
I could prepare PR to change that if you can confirm that it is possible to run the Selenoid browser images without that capability.

@Trane9991 could you briefly describe what exactly would you like to change first?

So, for now, I managed to build a workaround that works for now with selenoid -disable-docker binary running in the headless chrome from zenika/alpine-chrome docker image.
However, I have an idea of introducing -restricted-docker-mode flag for selenoid which will run containers following all Docker restrictions from Bitbucket Pipelines (and I think all other Docker-based CI systems), namely:

Run without privileged mode
Run without any added system capabilities
Do host volume mounts as the subdirectories of the current directory only
Add additional env variable as SELENOID_FLAGS to the Dockerfile, because bitbucket-pipelines does not allow to modify executed command for the service containers, only setting variables.

However, this is just an idea, that needs to be tested that it will "really" work, and since I managed to build a workaround I'm not going to work on this right now, maybe will pickup somewhere later.
Thanks, let me know what you think.

@Trane9991 ,
I have the same problem. Can you share your Bitbucket Pipelines, please? And one more question: did you try running tests in other browsers in Bitbucket Pipelines using selenoid?

@Trane9991 ,
I have the same problem. Can you share your Bitbucket Pipelines, please? And one more question: did you try running tests in other browsers in Bitbucket Pipelines using selenoid?

@Eleno4kaLap I ended up running docker-compose in bitbucket-pipelines.
version: "3.8"
services:
  selenoid:
    image: zenika/alpine-chrome:86-with-selenoid
    volumes:
      - "/var/run/docker.sock:/var/run/docker.sock"
    ports:
      - "4444:4444"
  tests:
    image: "gradle"
    command:
      - command
      - to
      - run
      - tests
    environment:
      - SELENOID_URL=http://selenoid:4444
    depends_on:
      - selenoid

then, in bitbucket pipelines the following step:
      name: Selenium Tests
      image: docker/compose
      script:
        - docker-compose up  --build --exit-code-from tests
      services:
        - docker
      caches:
        - docker
      artifacts:
        - video/**
        - screenshots/**
        - build/test-results/**
        - build/reports/**


✨(ci) add security scan

Add a security scan for CVE with trivy

See #500 for a version of this PR without the fixes applied, showing the Trivy report with CVEs.

After discussion with @mjeammet a good solution would be to make this an optional check in CI. Will look into that.


Reuse evdev/X11 keymap for wayland

So if I understand correctly the patch in hybrid environment where there's X and wayland (namely gnome3) your patch allows to find the keymap from the X environment even if we're running a wayland app ?

@freerdp-bot can you test that ?

This is actually for a pure wayland environment, for which there was no keycode conversion at all previously.
This uses the same keymap as X11, which is easy to do since the X11 keycodes are the Linux kernel (evdev) keycodes + 8.

That's a long time I've not played with wayland and keyboard, but from what I remember there was the need to look at the mapped xkb file sent to us by the compositor, so that we can send the correct RDP keyboard to the server. But there's no easy way to do that.
I may be wrong, but if your code works for you it may be because:

you have the default US keyboard;
with your changes, we detect the keyboard layout from the X emulation of your compositor, and by chance (or purpose) it's the same layout that the one used by the wayland part. Can you tell what kind of test environment you're using ?

Actually, in my case, this is with a TV remote control, so the keyboard mapping does not matter that much. But yes, it would default to a US keyboard mapping.
A better way would be to handle the compositor keymap, use the xkbcommon library, and write a xkb -> RDP mapping.
Even if not totally correct, I think this change is still an improvement, since previously the wayland keyboard mapping looked pretty random.
With this change, the wayland client is on par with the X11 client when libxkbfile support is not enabled.

So I will test on F25 which has gnome3 with wayland and X to see if it improves things with my french keyboard.
Basically we would need to know the features (layout, variant ) of the xkb file that is sent to us by the compositor. So that we can find a RDP layout that matches (as much as possible) the xkb file, the problem is that in wayland, we get a mapped file and we have lost all the informations about that file. So using X to know which file we get could be a hack to achieve the final goal (but would not work in environment where there's no X support at all).

So I've tested under GNOME3 under wayland (so a mixed environment that also have X), and connecting with wlfreerdp to a windows hosts with my french keyboard works. I will have a test with your patch applied to see if it still works.

@hardening Any update on this?


fix: use site URL without language code as base URL

Hello again, thanks for your last refactoring! Still got the language url fragment twice. This PR fixes the siteUrl fallback by using $kirby->url() instead of $kirby->site()->url() (based on this answer)

Good catch! 🔥


add support for multiple substrate urls

Right now rmb accepts a single chain (substrate) urls which is used mainly for twin queries to verify signatures. We need to be able to provide multiple urls in case one of them is down. Hence rmb can then try multiple backends in case one of them is failing

updated implementation to accept multiple substrate urls, but still needs to be tested.

@MarioBassem this can be easily tested because we already have multiple public nodes for mainnet
check here
https://github.com/threefoldtech/zos/blob/main/pkg/environment/environment.go#L113


Outlook mail App on iOS

This plugin works great for sharing things via email with standard iOS mail. I have a customer using OutLook app as their mailer,  and Outlook is not started when I call  shareviaemail().  Any suggestions on how to help them?
Jeff

Dunno, never used that app. If anyone has a suggestion, please reopen.


[Block] Columns (Features)

Implement Columns block with Features variant

[x] Build Mobile first
[x]  Tablet version
[x]  Desktop

See Samples at

https://maestro.maark.com/
https://maestro.maark.com/customer-stories

Fixed as part of #36


Added: Add playlist tag validation to ToontjeHoger(Kids)5Tempo rules

This pull request adds tag validation to the ToontjeHoger(Kids)5Tempo rules. The validate_tags method has been implemented to check if the tags in the playlist are valid. Additionally, a test case has been added to ensure that the validate_playlist method does not raise any errors for invalid tags.
Resolves #1076

One thing I wonder about: if we attach a playlist with 100s of incorrectly tagged sections, won't this mean the admin interface may actually get too crowded with warnings for tags? Perhaps it's better to just raise one error if any of the tags are invalid, which just states (in natural language) the naming convention for the tags?


Add support for new padding and margin property in React Native 0.51+

As you can see in this release note, React Native now supports for padding and margin. It seems Styled Components still doesn't support it.
Version
styled-components: 3.1.6
Reproduction
const RoundButton = styled.View`     borderRadius: 8;     background: ${ props => props.background || 'black' };     padding: 5; `;
Expected Behavior
It should works as you can see it in React Native's documentation
Actual Behavior

This has been supported for a while by css-to-react-native, which is a sub-library to convert more web-like css rules to React Native compliant CSS.
padding: 5 however needs a unit to be parsed correctly, meaning padding: 5px; will work just fine. This is to differentiate it with the ambiguous padding: 5%, which we've introduced to do some conversions more easily.
If we indeed don't need to convert this to padding{Top,Right,Bottom,Left} anymore, we still can't quite remove it, so that we support older versions of React Native.
cc @jacobp100

There was always support for padding on its own, we expand it to match css behaviour. I think this is more about paddingStart/paddingEnd, which change with writing direction
We do, however, need to look at multiple border colours and styles now being supported

I tested this: padding: 5px and it works. IMO it's better to support unitless values for shorthand CSS too, because React Native is basically unitless. It makes users confuse.

@mojtabast What's better is of course always subjective and relative, and you do have an argument.
But we have discussed this at length, and apart from the strongest argument—imho—is that we're trying to make things more similar to the web (like when we're emulating how flex works on the web). And then come the technical reasons of why this makes css-to-react-native's job easier.

I have also stumbled over this topic.
So basically
margin: 6;
cannot been parsed, but needs to be
margin: 6px;
But  margin-top: 6; just works fine. I think it would be better to have some consistence here.

That's more of an implementation detail in css-to-react-native. We're going to try and get some dev warnings for these cases

+1


chore(deps): upgrade dependencies

Upgrades project dependencies. See details in workflow run.

Automatically created by projen via the "upgrade-1.x" workflow

⚪ Backport skipped
The pull request was not backported as there were no branches to backport to. If this is a mistake, please apply the desired version labels or run the backport tool manually.
Manual backport
To create the backport manually run:
backport --pr 573

Questions ?
Please refer to the Backport tool documentation


Wither skeleton skulls not dropping when using prevent breaking

Killed 150 wither skeletons with Looting III and no wither skeleton skulls dropped. Disabled prevent breaking and got a skull after just a few skeletons. Might just be random chance, but seems incredibly unlikely.

Close as not reproducible


TS Server crashes at startup

Vue - Official extension or vue-tsc version
v2.0.22
VSCode version
1.90.2
Vue version
3.4.27
TypeScript version
5.4.5
System Info
System:
    OS: Windows 11 10.0.22631
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
    Memory: 14.24 GB / 31.74 GB
  Binaries:
    Node: 20.12.2 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.22 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 10.5.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 126.0.6478.114
    Edge: Chromium (126.0.2592.68)
    Internet Explorer: 11.0.22621.3527

Steps to reproduce
On startup the ts server will crash if the vue extension in enabled.  I can confirm it's crashed as there are no type errors in the project.   I'm making the repo public.  Nothing serious in it.
I'm running a clean install of vs code.  I deleted the user profile and reinstalled vs code.  Vue - Official is the only non @builtin extension installed.
Logs of the last start up are attached.
TypeScript.log
tsserver_2.log
tsserver_3.log
Link to minimal reproduction
https://github.com/jmarshall9120/advanced-storage-site-2/tree/dev
Any additional comments?
No response

I assume you're on the extension version 2.0.22?

@davidmatter - yes, I will correct the ticket.

I've had a short look at your repo: Please provide a minimal example. There's too many things in there to be able to debug properly.

On startup the ts server will crash if the vue extension in enabled. I can confirm it's crashed as there are no type errors in the project

There can be many reasons why no type errors are thrown to you and it doesn't automatically mean that the TS server crashed.

I've had a short look at your repo: Please provide a minimal example. There's too many things in there to be able to debug properly.

On startup the ts server will crash if the vue extension in enabled. I can confirm it's crashed as there are no type errors in the project

There can be many reasons why no type errors are thrown and it doesn't automatically mean that the TS server crashed.

包括我在内，以及我身边的同事在同样的版本下，也出现了无法正常使用 ts 的情况， 在我关闭了 vue-offical 扩展后，ts 就能正常使用了

I think this is related to this issue: #4497

@davidmatter - I'm traveling on vacay, so I won't have more on this for a couple weeks.
Can u provide for me some tips on better debugging language service issues?  What I have done:

CPU Profiled the plugin.
Set TS output to verbose. Looked through the output logs.
Tested on clean install of VS studio

The project stack is:

Vue.js
Vuetify
AWS Amplify
NUXT.js

I could start a fork where I start ripping out elements of the stack and testing the ts performance, but I'm not sure where that would land me.  If I can't see the interaction the ts server is having now that is causing it to shutdown, and I do I identify an element of the stack that is involved in the issue, I'd still have know idea what the conflict is.
Any info about the tooling or further techniques to debug would be appreciated.

For anyone facing this issue and willing to assist us in investigating via remote desktop, please see: https://github.com/vuejs/language-tools/wiki/Help-Us-Investigate-Issues-Remotely

@johnsoncodehk - I'm working on making this more reproducible.  I'll share another repo, soon and we can we can remote over that one.  In the mean time, can you confirm whether this is or is not normal behavior:

I believe the abc prefix on those images means the intellisense is resolving from text parsing the files and not symbols parsed by the vue language server.  Those should look like this:

Am I correct in my assessment here?


1.5.8 is not tagged on GitHub

On PyPI, 1.5.8 is the latest version. However, this version is not tagged on GitHub. It looks like it might have been tagged as 1.5.7 instead.

I may be cutting a new release soon, so I'll address that at that point.

Sorry to dig up this old issue.
Would you consider tagging the 1.5.8 release on the repository or maybe make a new release?
I'm the Debian maintainer of this package and there is a discrepancy between the public version (1.5.7) and the version reported by pip (1.5.8).

Sorry to dig up this old issue.
Would you consider tagging the 1.5.8 release on the repository or maybe make a new release?
I'm the Debian maintainer of this package and there is a discrepancy between the public version (1.5.7) and the version reported by pip (1.5.8).


RHIDP-4660: updating line to reduce redundancy across docs

IMPORTANT: Do Not Merge - To be merged by Docs Team Only
Version(s): 1.3.x, main
Issue: RHIDP-4660
Reviews:

[ ] SME: @nickboldt
[ ] QE: @psrna
[ ] Docs review: @Gerry-Forde

/lgtm