@swm/global nav menu v1

Details

The v1 version of the global navigation. Only one option "Chats" is included.
Fixed Issues

$ https://github.com/Expensify/App/issues/27000
PROPOSAL:
Tests

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

Offline tests

QA Steps

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

PR Author Checklist

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

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

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

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

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

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
[x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method

[x] If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:

[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

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

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

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

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

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

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

[x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
[x] I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Web
https://github.com/Expensify/App/assets/67908363/2b1c6f29-5af1-4962-acf2-105c7d5f20e8

Mobile Web - Chrome

Mobile Web - Safari

Desktop
https://github.com/Expensify/App/assets/67908363/54764842-7280-48d8-b45a-e506ad08eae7

iOS

Android

@shawnborton

These are all of the icons we'll need, some already exist in the App repo though, so you might need to upload certain ones:

Icon svgs: Icons.zip

@allroundexperts I made some adjustments.

Bug: The chat menu is not highlighted on mWeb.

This also should be fixed

It looks like something weird happened to the header 🤔 If you have any idea why please let me know. I will look into it on Monday

@allroundexperts do you have any idea how to solve this?

Note:
If the new URLs (ex. home and expenses) are entered into URL, currently the default 404 page "Hmm... it's not here" is displayed. I think this is the expected behavior as these screens haven't been implemented yet (i.e. iframe has not been added yet)

@adamgrzybowski
also, I might have said otherwise before, but we also want to display Global Nav and Sub Nav even on mobile web.
I can confirm that Global Nav and Sub Nav are displayed on iOS. Can you do the same thing for mobile web?

Currently, when the browser width shrinks, both Global Nav and Sub Nav disappears.
https://github.com/Expensify/App/assets/98560306/6063a09e-dea2-43a3-99f1-bbcd5b119217

@stitesExpensify I know you're working partially right now but I assigned you as a reviewer in case you also want to review this PR that adds the Global Nav to NewDot.

Currently, when the browser width shrinks, both Global Nav and Sub Nav disappears.

https://github.com/Expensify/App/assets/67908363/956f0fc1-f06f-4d18-a293-ed1bab9cd86a
Isn't this the same as in the current implementation?

Isn't this the same as in the current implementation? @hayata-suenaga

I should have made this more obvious, but the requirements were communicated previously. Please see the screenshots
The screenshot for mobile also applies to mobile web.

For this shot here:

I kind of think we want the top status bar area to match the left global nav area. Thoughts @Expensify/design ?

Isn't this the same as in the current implementation? @hayata-suenaga

ah I see. I can see the list of chats on the sub nav and the "Chats" menu on the global nav (I forgot that a report number is automatically appended to the URL by default and shrinking the window size results in the report view displaying that report instead of SidebarScreen.

@allroundexperts @hayata-suenaga a new wave of adjustments!

@adamgrzybowski its flaky test, can you please sync up with main, we skipped those, I will then create the build

Running the test build here

Use the links below to test this build in android and iOS. Happy testing!

:test_tube::test_tube: Use the links below to test this build in android and iOS. Happy testing! :test_tube::test_tube:

android :robot:
iOS :apple:

https://ad-hoc-expensify-cash.s3.amazonaws.com/android/28277/index.html
https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/28277/index.html

desktop :computer:
web :spider_web:

https://ad-hoc-expensify-cash.s3.amazonaws.com/desktop/28277/NewExpensify.dmg
https://28277.pr-testing.expensify.com

Woo, looking amazing! Going to leave a bunch of feedback for you here.
It looks like you might have made some kind of global change that is effecting the padding on our select menu items. Notice how these are shifted to the left a bit:

Minor, but let's add like 4-8px of spacing between Chats and the environment label:

I think you need to merge main into this branch as I am not seeing recent updates to the Settings page which has a highlighted top area:

The LHN rows do not have any :hover effect. Maybe we can just use the same highlightBG color on :hover for the background?
For the LHN rows, let's have them use a border radius of 8px.

Also cc @Expensify/design in case you wanna test the links above.

Can we make the global create menu a bit closer to the FAB button? Perhaps just 12px away from it would be good?

For smaller viewports, I am seeing a slight little jumpyness when you navigate into a chat:
https://github.com/Expensify/App/assets/2319350/c728863d-8491-4bfe-b1cb-ab64cd778c87
Almost like it first goes up and then over to the left.

Not sure if this is worth addressing, but for some reason the composer gets borked when we resize the window.

For smaller viewports, I am seeing a slight little jumpyness when you navigate into a chat:

Do you think it is specific for this PR or the main behaves similarly? @shawnborton

Do you think it is specific for this PR or the main behaves similarly?

I think you are right, this already exists. We can ignore.

@hoangzinh I'm unassigning you as you already said you have other PRs to review. But let me know if you're interested reviewing this PR

@shawnborton It looks like sidebar links on the left and right share some styles (e.g. padding and background color when active). In this PR, the background color for active links on the left was changed. Should this also affect the sidebar links on the right? Or maybe should I separate these styles?

I think it's worth separating those styles for now

@shawnborton new style adjustments 👨🎨

Let's see if we can spin up a new build!

Can we spin up another test build? I tried to reapply the label but I don’t
think it worked.
On Wed, Oct 4, 2023 at 4:29 AM Hayata Suenaga @.***>
wrote:

@.**** commented on this pull request.
In src/libs/Navigation/getTopMostCentralPaneRouteName.js
https://github.com/Expensify/App/pull/28277#discussion_r1345409856:

@@ -0,0 +1,32 @@
+import lodashFindLast from 'lodash/findLast';

+/**

Find the name of top most central pane route.

@stitesExpensify https://github.com/stitesExpensify good point! Let's
use ContentPane, @adamgrzybowski https://github.com/adamgrzybowski
—
Reply to this email directly, view it on GitHub
https://github.com/Expensify/App/pull/28277#discussion_r1345409856, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AARWH5UKZCU6F37DBLUCQR3X5UM7ZAVCNFSM6AAAAAA5IFBAFCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTMNJWHE3DIOBWGE
.
You are receiving this because you were mentioned.Message ID:
@.***>

@adamgrzybowski I realized you're still using CentralPane in several places. Can you update them to ContentPane?

@shawnborton Running the test build here. The QR codes and link should be available in about an hour

I realized you're still using CentralPane in several places. Can you update them to ContentPane?

I am a little confused by that. When did this change happen and why? @hayata-suenaga

:test_tube::test_tube: Use the links below to test this build in android and iOS. Happy testing! :test_tube::test_tube:

android :robot:
iOS :apple:

https://ad-hoc-expensify-cash.s3.amazonaws.com/android/28277/index.html
https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/28277/index.html

desktop :computer:
web :spider_web:

https://ad-hoc-expensify-cash.s3.amazonaws.com/desktop/28277/NewExpensify.dmg
https://28277.pr-testing.expensify.com

Hmm I'm not seeing any style changes on the new build?

Okay going to just pull this locally instead...
For the LHN rows, looks like we are now back to having too much horizontal padding:

Those should have 8px horizontal padding and 12px vertical padding:

And they should be wrapped in a container that has 12px horizontal padding.

Not sure if this is related to this PR or not, but we need to update border colors of our avatar component when rows are highlighted or hovered:

Seems like our alignment is off slightly too. I wonder if we should nudge up the global nav bar and the start of the LHN rows so that they sit more evently with the bottom of the content area's header bar?

By how much? Like 2px? @shawnborton
BTW LHN rows should be fixed now

Yup, looks like 2px or so. And then can we make the Chats text vertically centered with the avatar to the left? I think the Chats text is wrapped in padding that's like 24px top, 16px bottom. So let's just even that out to be 20px top and bottom.

Bug: Merge conflicts double <SignInOrAvatarWithOptionalStatus />

@shawnborton @s77rt  Not sure what to do about this

For the dark theme the right color is theme.lightText  but for the light theme theme.textDark looks like a better option. But that would be inconsistent. Any ideas?

For the dark theme the right color is theme.lightText but for the light theme theme.textDark looks like a better option. But that would be inconsistent. Any ideas?

@adamgrzybowski you should just be using the standard text and textSupporting colors here. They will automatically flip when we change themes.

@shawnborton the textSupporting for the dark theme is AFBBB0 and on the designs it is E7ECE9. Should I use it anyway?

Yes, you should. I'm not sure where you are getting that value from, in Figma the values match text and textSupporting.

Just to make sure, I am talking about the focused version
in default.js
textSupporting: colors.darkSupportingText,
in colors.js
darkSupportingText: '#AFBBB0',

That is the correct value for supporting. But in the image you shared, we are not using supporting but rather we are just using our standard text color.

Perfect, thank you!

I've used custom values:

13 for global nav margin
19 for the header vertical padding

but now it looks aligned.

I'll wait for @s77rt's review before merging
and for the final sanity check, I'm rebuilding the test build here. @shawnborton could you do the final round of design review to make sure everything is okay?

Not a blocker (and maybe intended? cc @shawnborton) the items in the global navigation do not have a hover effect
https://github.com/Expensify/App/assets/16493223/5b001607-cc9e-404b-92de-adc080eef911

The search icon is misaligned

Minor bug: on Desktop, the avatar is too close to the window buttons. Also the window buttons background is split (two backgrounds)

Reviewer Checklist

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

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

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

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

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

[X] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
[X] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[X] I verified that comments were added to code that is not self explanatory
[X] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[X] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
[X] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[X] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[X] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[X] I verified the JSDocs style guidelines (in STYLE.md) were followed

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

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

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

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

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

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

[X] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[X] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[X] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[X] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[X] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[X] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
[X] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

Cool, I have some design feedback so hang tight while I put together some screenshots!

On mobile web, can we make the top status bar area match the color of the global nav? So make it darker:

Mobile web has a weird loading screen, is that a result of this PR?

We still have an incorrect border color on the second avatar here:

Similar comment about making the search icon further to the right to line up perfectly with the pin icon:

Are you sure you have pulled main recently? On main, the Settings page should look different.

Your Branch
Production

Same comment about the desktop top buttons:

I think those should sit within the lighter green area and push the avatar down slightly. And then we'd have a small space above the "Chats" header as well as above the Content pane header.

I can't seem to get iOS to install. @mountiny any ideas there? It says the integrity can't be verified when I try to open it on my phone.

I feel like at one point it did, yeah.

Added your device to the provisioning profile, hopefully it will work next

:test_tube::test_tube: Use the links below to test this build in android and iOS. Happy testing! :test_tube::test_tube:

android :robot:
iOS :apple:

https://ad-hoc-expensify-cash.s3.amazonaws.com/android/28277/index.html
https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/28277/index.html

desktop :computer:
web :spider_web:

https://ad-hoc-expensify-cash.s3.amazonaws.com/desktop/28277/NewExpensify.dmg
https://28277.pr-testing.expensify.com

Dang, still having that same integrity error.

Ok it works for me, I will ask in Slack what else is required.

@mountiny asked the question about provisioning profile here

Started a build with new provisioning profile 🤞

:test_tube::test_tube: Use the links below to test this build in android and iOS. Happy testing! :test_tube::test_tube:

android :robot:
iOS :apple:

https://ad-hoc-expensify-cash.s3.amazonaws.com/android/28277/index.html
https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/28277/index.html

desktop :computer:
web :spider_web:

https://ad-hoc-expensify-cash.s3.amazonaws.com/desktop/28277/NewExpensify.dmg
https://28277.pr-testing.expensify.com

Dang, I still can't install iOS.

Dang, I am not sure whats next, this should have the new profile with your deviceID in it

Hey, I see there are still a few style related things to fix. I would like to propose to implement them after the rest of the global navigation logic. There are a few factors why I am considering it:

While all those changes are mostly small things I think they can add up.

Until this moment my work was mainly related to navigation. I don't have much experience with existing components and conventions in the context of Expensify. That may cause additional delays.

I think that we can relatively safely work on styles after finishing the logic without worrying that these changes will affect the behavior of the app. That means we can test the logic and work on styles in parallel.

Finally, we have a pretty close deadline so I think it may be worth focusing on functionality first.

Of course, I can understand the reasons for merging only style perfect PR with the main. On the other hand, it's not about ignoring all these valuable comments on how to improve the look of the app. It's about moving them later in the production cycle.
I would be happy to hear your opinion on this. I think this option is worth considering but I will be happy with any decision.
cc: @hayata-suenaga @shawnborton @s77rt @allroundexperts  @mountiny

I added the QA test steps in the OP.

:hand: This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

🚀 Deployed to staging by https://github.com/hayata-suenaga in version: 1.3.80-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.80-3 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
failure ❌

🕸 web 🕸
success ✅

🚀 Deployed to staging by https://github.com/hayata-suenaga in version: 1.3.81-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.83-11 🚀

platform
result

🤖 android 🤖
skipped 🚫

🖥 desktop 🖥
skipped 🚫

🍎 iOS 🍎
skipped 🚫

🕸 web 🕸
skipped 🚫

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.83-11 🚀

platform
result

🤖 android 🤖
skipped 🚫

🖥 desktop 🖥
skipped 🚫

🍎 iOS 🍎
skipped 🚫

🕸 web 🕸
skipped 🚫

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.83-11 🚀

platform
result

🤖 android 🤖
skipped 🚫

🖥 desktop 🖥
skipped 🚫

🍎 iOS 🍎
skipped 🚫

🕸 web 🕸
skipped 🚫

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.83-11 🚀

platform
result

🤖 android 🤖
failure ❌

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
failure ❌

🕸 web 🕸
success ✅

🚀 Deployed to production by https://github.com/francoisl in version: 1.3.84-10 🚀

platform
result

🤖 android 🤖
skipped 🚫

🖥 desktop 🖥
skipped 🚫

🍎 iOS 🍎
skipped 🚫

🕸 web 🕸
skipped 🚫

🚀 Deployed to production by https://github.com/francoisl in version: 1.3.84-10 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
failure ❌

🕸 web 🕸
success ✅


Discussion: Remove Package.resolved from repository

It's a real pain for development having the Package.resolved file in source control. It is autogenerated so shouldn't need to be included.
If you have a dependency on a branch name you keep having to update the Package.resolved file to get it to build against latest

I think it's a good idea. As a library, it makes sense for dependencies to be pinned in Package.swift.

I'm slowly catching up on WWDC videos, and toward the end, this one suggests checking in Package.resolved. It looks like there's also some discussion on the forums as well.
I think this advice is mainly for end-users (of an application), the majority of folks who'd run into the Package.resolvedfor this library are likely going to be actively developing this project. There's some value in folks knowing they're all on the same version, but given how active development is (and a few branches in flight) I think making development as easy as possible is wise.
All in all, I support this 👍

@Yasumoto Just to be clear when I added this I meant it for aws-sdk-swift. When developing features that use both repositories that is when the Package.resolved gets in the way the most.
I understand the value in ensuring folk are using the same version, but we should be able to edit the Package.swift to ensure that.

Package. resolved has been removed from aws-ask-swift and added to the gitignore file


UI does not refresh automatically

Hello,
My componnets are not refreshing. I need to click on the component for exemple header of the datatable in order to view table content (refresh), or click on the input text in order to refresh content.
I am using the latest version 8-1 with angular4 and material2.beta12 :
below is my code :
ngOnInit() {
this._titleService.setTitle( 'Users List' );
this.accountApi.find(this.lbFilter).subscribe(users => {
this.data = users;
this.filteredData = this.data;
this.filteredTotal = this.data.length;
this.filter();
});
}
filter(): void {
let newData: any[] = this.data;
let excludedColumns: string[] = this.columns
.filter((column: ITdDataTableColumn) => {
return ((column.filter === undefined && column.hidden === true) ||
(column.filter !== undefined && column.filter === false));
}).map((column: ITdDataTableColumn) => {
return column.name;
});
newData = this._dataTableService.filterData(newData, this.searchTerm, true, excludedColumns);
this.filteredTotal = newData.length;
newData = this._dataTableService.sortData(newData, this.sortBy, this.sortOrder);
newData = this._dataTableService.pageData(newData, this.fromRow, this.currentPage * this.pageSize);
this.filteredData = newData;
}
thankyou in advance for your help

I found a solution but don't know if it the better one :+1:
I am using ChangeDetectorRef and calling  the .detectChanges(); method within the ngOnInit after loading data..
Do you think that what i did is a good solution ?
Thank you in advance

This is probably a change detection issue from the implementation side since you had to call markForCheck. Closing this since there was a solution


Add the Azure Boards badge to the README

Add the Azure Boards badge for the board used to track the work for this repository. Fixes AB#509. See the status badge configuration documentation for more information.

OK. Badge gets added to readme.


qr code page

@keybase/react-hackers
Do 'already on keybase? login'
Connect another device
Phone
You'll see

@keybase/react-hackers  bump

:thumbsup:


Why is 'createdump' included in dotnet publish builds?

Release builds made by dotnet publish include createdump in Linux - what is it for and why would we want to distribute it?

See https://github.com/dotnet/runtime/issues/43716.


Improve some Japanese translations

In this pull request, some Japanese translations are improved.
I changed some Japanese translations that I found a little bit disgusting.

Thank you :)


Model Parallelism

Hey,
Is it possible to use accelerate with model parallelism?
If so, can you provide an example?

No, Accelerate only supports data parallelism to train on multiple GPUs or TPUs for now.

@sgugger Maybe we can put a proper prompt in the readme or documentation so users can be aware that currently only data parallelism is supported?
Hope Accelerate will support model parallelism in the future!


Feature/shopping list

Implemented Features
General requirements

Make sure that there are no linter errors.
Make sure that you used the correct Gitflow.
Make sure that you documented your work in a professional way.

Ruby requirements

Follow our list of best practices for Ruby.

Project requirements

changes
#30


Removed duplicate content guide

What is the context of this PR?
There was a dupe of the 'writing page titles and urls' content guide under /headings
How to review
Check dupe page is removed.

 Fix multiple content guides issue


Add hidden winget complete command to enable tab-completion

Description of the new feature/enhancement

The dotnet cli tool from the .NET Core SDK has a hidden complete command that works like this:
PS> dotnet complete --position 2 "re"
remove
restore
store

This can be hooked into environments like PowerShell to enable command/parameter completion for dotnet.exe e.g.:
Register-ArgumentCompleter -Native -CommandName dotnet -ScriptBlock {
    param($wordToComplete, $commandAst, $cursorPosition)

    dotnet complete --position $cursorPosition $commandAst | ForEach-Object {
        [System.Management.Automation.CompletionResult]::new($_, $_, 'ParameterValue', $_)
    }
}

Proposed technical implementation details (optional)

I would like to see winget implement a hidden complete command that would provide completions for top-level commands e.g.:  install, show, source, search, hash, validate.  And then for each command, it should provide parameter completions e.g. winget install --id Git.Git --s should return --source and --silent.

Relates to #146.

Closing issue as this has been released, see the doc for more details.


Carthage support

Any chance for Carthage support?

No plan to support Carthage at this point, but I'll leave this open for feedback.

I just updated a project that I was using to be able to download GoogleSignIn with Carthage, you might be able to use it as well (or use your own fork of it).
Perhaps Google can create a similar wrapper framework and add it to this repository, then it'd make things more convenient.

+1

Just use Firebase Carthage manual.
Add carfile
binary "https://dl.google.com/dl/firebase/ios/carthage/FirebaseGoogleSignInBinary.json"


If optional keyfile setting isnt defined the plugin fails

Related to slack thread https://meltano.slack.com/archives/C01TCRBBJD7/p1664397294675829.
It looks like the file bundle installs a profile that has an optional keyfile setting, if its not provided then it uses an empty string as default but that causes a No such file or directory: '' error.
This profile will be moved to https://github.com/meltano/dbt-ext/blob/feat/working-dbt-ext/files_dbt_ext/profiles/bigquery/profiles.yaml soon.

I can comment that it is not optional. I specify big query to use service-account method which means it's required.
but this env var is not getting set with the file path used by target bigquery


[1.15.2] Crash on Startup

Mod Version: 2.1.0
Forge Version: 31.1.18
Relevant piece of log: https://pastebin.com/mm605GBB

Yeah, the mod does not currently support 1.15.2, apparently Forge made breaking changes. I have been too busy to put much time into modding for awhile.

Is there anything I can do while I wait for a fix?

This was fixed by #24


Fix model binding testing setup to use the dependent components consistently

Example: Here we create options but this is not present in DI services as IOptions<MvcOptions> service.
Example: In this test, we use different instances of services when getting a ParameterBinder and when getting  a TestContext.
Just like how an application in general starts with configuring services, and later requests are executed, we should setup the DI container with all the required services in the test and use this across. I can imagine the usage experience might become a bit undesirable but at least we would have the setup correct.
cc @dougbu

Closing this. We can always clean up our tests as we go


Support for HTTP wait strategy

Are there any plans to support the HTTP wait strategy? https://www.testcontainers.org/features/startup_and_waits/#other-wait-strategies

It should be relatively straightforward to support http wait strategies using a generalisation of wait_for_logs. PRs welcome!

I took a stab at adding the port and http status code wait strategies: https://github.com/smparekh/testcontainers-python/compare/30dce29920766e3d34926d4ec174ab11f6158f81...smparekh:master
Comments and suggestions welcome.


Added Lowest Common Ancestor of a Binary Tree

This is the solution for #202  issue.

Hi @vashuteotia123  , I have sent a pull request. Please review it and merge it in main branch and mark this issue as solved if possible.

Is this repo not a part of hacktoberfest


Additional includes needed when library is used outside of .ino file

To reproduce:
Create a new project with additional .c/.cpp files. Include the BLE library in the project, and copy all those includes to one of the .c files as well. Make calls to the BLE library from that external .c file. Project will fail to compile, complaining of unknown types. These includes get the type definitions in the right place for successful compilation.
There might be a better way to handle this case, but at any rate, this worked for me, and I just thought I'd share.

 Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.Don Goodman-Wilson seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.You have signed the CLA already but the status is still pending? Let us recheck it.

The email address used for this PR is long dead, thus I have no way of proving I am<EMAIL_ADDRESS>anymore. I'm going to close this PR—anyway, it's five years old! Who knows if this is even a problem anymore.


Automatic CI

Github Actions is free for public repositories (like this). If you want I can create an automatic pipeline which creates build artifacts on each release.

Ah nevermind. Looked somehow (don't ask how) into the wrong repo.


Build failure: Space Repo exceeded monthly outgoing traffic

It looks like the Space repo containing the artifacts served to much traffic this month...
https://maven.pkg.jetbrains.space/public/p/compose/dev/org/jetbrains/compose/runtime/runtime/1.0.0-beta6-dev450/runtime-1.0.0-beta6-dev450.pom
Results into 429: {"error":"Packages monthly outgoing traffic limit of 1.4 TB exceeded"}
Because Gradle cannot download the runtime artifact, it is impossible to build any Compose projects.

We are looking into it

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.


Error installing react-native-onesignal on fresh RN project

Hi @avishayil ,
Just wanna follow up issues that i've posted before. #129
Could you please check it out?
Thank you

Let's continue there.


Feature/2320 - Update Menu endpoint and unit tests

2320 - Update Menu endpoint and unit tests
📲 What
Implemented an endpoint to update an existing menu
🤔 Why
To update an existing menu
🛠 How
Can update an existing menu by the menuId
👀 Evidence
Screenshots / external resources / links / etc.
Link to documentation updated with changes impacted in the PR
🕵 How to test
Please check the user story for Acceptance Criteria
✅ Acceptance criteria Checklist

[ ] Code peer reviewed?
[ ] Documentation has been updated to reflect the changes?
[ ] Passing all automated tests, including a successful deployment?
[ ] Passing any exploratory testing?
[ ] Rebased/merged with latest changes from development and re-tested?
[ ] Meeting the Coding Standards?

Closing these changes are merged into master in https://github.com/amido/stacks-java/pull/34#pullrequestreview-454348588


Update tests for src/vessel-charging/messages/ChargingCompleteMessageParams.test.ts using jest (first-timers-only)

first-timers-only
This issue is tagged :octocat: first-timers-only. It is only for people who have never contributed to open source before, and are looking for an easy way take their first steps.
Consider this your chance to dip your toe into the world of open-source, and get some bragging rights for writing code that makes drones fly, lets cars find charging stations, helps people and goods get from place to place, and more.
Find more first-timers-only issues from DAV Foundation here.
Thank you for your help :heart:
What is this project?
DAV (Decentralized Autonomous Vehicles) is a new foundation working to build an open-source infrastructure for autonomous vehicles (cars, drones, trucks, robots, and all the service providers around them) to communicate and transact with each other over blockchain.
As an organization that believes in building a large community of open-source contributors, we often create issues like this one to help people take their first few steps into the world of open source.
dav-js
This repo contains the DAV JavaScript SDK. This SDK allows developers to build applications and servers that connect to the DAV network. For example, allowing a drone to find charging stations, or an autonomous car to ask for traffic data.
How you can help
In order to foster a community that is welcoming for open source contributions, it is important for us to have good test coverage. And good tests are simple, readable tests.
Here is a good opportunity to simplify one of our tests.
The Issue
In src/vessel-charging/messages/ChargingCompleteMessageParams.test.ts we defined the following parameters at the beginning of the describe() block.
  const messageParams = new MessageParams({
    senderId: 'TOPIC_ID',
  });

  const serializedMessageParams: any = {
    ttl: undefined,
    protocol: 'vessel_charging',
    type: 'charging_complete_message',
    senderId: 'TOPIC_ID',
  };

These parameters will need to accommodate additional unit tests.
Please update these parameters to be more dynamic by defining them to be mutable similar to src/vessel-charging/MissionParams.test.ts.

Contributing to dav-js

[ ] Make sure this issue is labeled up-for-grabs and not labeled claimed, to verify no one else is working on it.
[ ] Comment in this issue that you would like to do it.
[ ] Open dav-js GitHub page and click the ★ Star and then ⑂ Fork buttons.
[ ] Clone a copy to your local machine with $ git clone git@github.com:YOUR-GITHUB-USER-NAME/dav-js.git
[ ] Code Code Code
[ ] Once you've made sure all your changes work correctly and committed all your changes, push your local changes back to github with $ git push -u origin master
[ ] Visit your fork on GitHub.com (https://github.com/YOUR-USER-NAME/dav-js) and create a pull request for your changes.
[ ] Make sure your pull request describes exactly what you changed and references this issue (include the issue number in the title like this: #7)
[ ] Please do not fix more than one issue at a time. Your pull request should only fix what is described in this issue.

Asking for help
We appreciate your effort in taking the time to work on this issue and help out the open source community and the foundation. If you need any help, feel free to ask below or in our gitter channel. We are always happy to help 😄

I'd like to claim this!

It's all yours @Wmccra 👍

I would like to claim it, it would be basically place the variables into the beforeEach() and run the tests to check that they are working properly, right?
  let messageParams: MessageParams;
  let serializedMessageParams: any;

  beforeEach(() => {
    messageParams = new MessageParams({
      senderId: 'TOPIC_ID',
    });

    serializedMessageParams = {
      ttl: undefined,
      protocol: 'vessel_charging',
      type: 'charging_complete_message',
      senderId: 'TOPIC_ID',
    };
  });

Thanks!

That is correct! This is all yours @VladmirVolgen 😄


[visual-editor] Line height on Activity Marker

Seems little bit off

Is that what you mean? It kind of reminds me of those rotating number displays:
https://www.gettyimages.com/detail/video/visual-display-of-1956-gas-pump-usa-stock-video-footage/102498883
...
Now I kind of want our counter to do the rotation 😆


fix the panic of SetupRoutes in csi-plugin

Description
When the IP allocation conflicts, and the network card name occupying the IP is inconsistent with the network card name of the Pod, it will cause panic.
The setupRoutes function will get all the routes and configure the IP. Default route does not have dst information(dst = nil), and referencing dst.IP directly will panic.
Related issues/PRs
ISSUE LINK: (https://github.com/projectcalico/calico/issues/5779)

 Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

/sem-approve

@caseydavenport
Is there a chance to cherry-pick this fix to v3.22.
We met this bug in v3.22.4

Put up a cherry-pick PR, but not sure when the next patch for v3.22 will be (just missed the boat on v3.22.4)


Installation fails with apparent Illuminate/support incompatibility (Laravel 5.6.7)

Laravel Version: 5.6.7
Voyager Version: ^1.0 (installing with composer require tcg/voyager)
PHP Version: 7.2.2
Database Driver & Version:5.7.21

Description:
Installing with Composer require tcg/voyager fails due to an apparent Illuminate/support incompatibility. See attached file for error message.
Voyager installation error.txt
Steps To Reproduce:
Attempt to install with composer require tcg/voyager

Voyager is not fully ready for Laravel 5.6.
However, you can use the v1.x-dev branch which is ready for it but not yet released.

You may be interested to know that v. 1.0.x-dev (couldn't find a 1.x-dev branch) also fails with Illuminate issues (see attached copy of the error messages). Thought you'd want to know as you work on the release. By the way, do you have any timeline on the release day for a 5.6 compatible release?
Voyager 1.0.x-dev install failure.txt

v1.0.* is the current active release (currently v1.0.16), so of course it does.  If you want to use the 1.x-dev BRANCH (not release), you have to set your Voyager version in composer.json to "dev-[branchName]" (dev-1.x-dev).  This is all covered in the composer documentation


Changed locker.empty() to locker.clear() in README

Nice catch thanks

No problem.  Love the library.  Thanks for sharing it!


Running examples by id does not work when examples are defined in a required file

Subject of the issue
In the MongoDB Ruby driver we have some tests which use a common runner on different data sets. Up to now the test definitions were copy pasted for each data set, which was not ideal. I decided to DRY up these definitions by creating a method which defined the tests. Unfortunately rspec seems to deal with this situation quite poorly.
I created a small project to demonstrate the issue: https://github.com/p-mongo/tests/tree/master/rspec-id. It has a file spec/foo_spec.rb which invokes define_example which itself is in spec/bar.rb. The define_example function calls describe/it to create the tests.
The first issue is it appears to be impossible to execute any of tests defined in this way by their id. If I run rspec (and thus the entire suite) I get:

speed% rspec
FFF

Failures:

  1) foo bars
     Failure/Error: expect(1).to eq(2)

       expected: 2
            got: 1

       (compared using ==)
     # ./spec/bar.rb:4:in `block (2 levels) in define_example'

  2) foo bazzes 0
     Failure/Error: expect(1).to eq(3)

       expected: 3
            got: 1

       (compared using ==)
     # ./spec/bar.rb:9:in `block (3 levels) in define_example'

  3) foo bazzes 1
     Failure/Error: expect(1).to eq(3)

       expected: 3
            got: 1

       (compared using ==)
     # ./spec/bar.rb:9:in `block (3 levels) in define_example'

Finished in 0.00778 seconds (files took 0.05593 seconds to load)
3 examples, 3 failures

Failed examples:

rspec './spec/bar.rb[1:1]' # foo bars
rspec './spec/bar.rb[1:2]' # foo bazzes 0
rspec './spec/bar.rb[1:3]' # foo bazzes 1

rspec --help says:

    You can also pass example ids enclosed in square brackets:

      rspec path/to/a_spec.rb[1:5,1:6] # run the 5th and 6th examples/groups defined in the 1st group

This does not work given the ids reported by rspec above:

speed% rspec './spec/bar.rb[1:1]'
Run options: include {:ids=>{"./spec/bar.rb"=>["1:1"]}}

All examples were filtered out

Finished in 0.00012 seconds (files took 0.03192 seconds to load)
0 examples, 0 failures

The second issue is the stack trace does not include foo_spec.rb at all. Running rspec -b I only get:

...
     # /home/w/.rbenv/versions/2.6.1/lib/ruby/gems/2.6.0/gems/rspec-expectations-3.8.2/lib/rspec/expectations/handler.rb:48:in `handle_matcher'
     # /home/w/.rbenv/versions/2.6.1/lib/ruby/gems/2.6.0/gems/rspec-expectations-3.8.2/lib/rspec/expectations/expectation_target.rb:65:in `to'
     # ./spec/bar.rb:9:in `block (3 levels) in define_example'
     # /home/w/.rbenv/versions/2.6.1/lib/ruby/gems/2.6.0/gems/rspec-core-3.8.0/lib/rspec/core/example.rb:254:in `instance_exec'
     # /home/w/.rbenv/versions/2.6.1/lib/ruby/gems/2.6.0/gems/rspec-core-3.8.0/lib/rspec/core/example.rb:254:in `block in run'
     # /home/w/.rbenv/versions/2.6.1/lib/ruby/gems/2.6.0/gems/rspec-core-3.8.0/lib/rspec/core/example.rb:500:in `block in with_around_and_singleton_context_hooks'
...

Since the define_example method can be called from multiple files, thus defining different examples, only having bar.rb in the stack trace is insufficient to determine which file caused the failure.
Your environment

Ruby version: 2.6.1
rspec-core version: 3.8.0

Steps to reproduce
https://github.com/p-mongo/tests/tree/master/rspec-id
Expected behavior
There should be a way to:

Run a test by its id, even if the test is defined in a file required from a spec file and not in a spec file itself.
See both the spec file and the file where the test is defined in the backtrace.

Actual behavior
Neither of the above appears to be possible.

:wave: First off, have you checked out https://relishapp.com/rspec/rspec-core/v/3-8/docs/example-groups/shared-examples theres a whole bunch of things we do to allow that to programmatically define examples in a user friendly way. It may be a better interface for what you're doing that inventing your own from scratch.
Secondly, your example is fixed simply by wrapping define_example in a proper describe:
# spec/foo_spec.rb
require 'rspec'
require_relative './bar'

RSpec.describe do
  define_example
end
# =>
# rspec ./spec/foo_spec.rb[1:1:1] #  foo bars
# rspec ./spec/foo_spec.rb[1:1:2] #  foo bazzes 0
# rspec ./spec/foo_spec.rb[1:1:3] #  foo bazzes 1

Why? The RSpec DSL uses the file it was called from to define its top level name, and then the lower contexts are chained off that, so you need at minimum a proper describe in each spec file.

Also in future by minimum reproduction repo we mean a repo with just the issue at hand, not a massive repo full of tests, to avoid downloading large amounts of unnecessary files I had to manually download your two files, if it had been larger I wouldn't have bothered.

Thanks, putting a describe block around the method call works. I could not figure out where in the documentation this was stated.

It's not stated, but you're not doing something we support, I think there's
a public api for creating example groups with name, if not I'd accept a pr
making a way to create them programmatically rather than abusing the core
dsl

I'm afraid I don't exactly know what "creating example groups with name" means but here is an example of programmatically building the specs from external test definitions that we use:
https://github.com/mongodb/mongo-ruby-driver/blob/80eb6f0185172dbc5866c1318ddf2667a2b11b53/spec/support/crud.rb

Yes that is using our DSL outside of its intended use. As a forewarning it will break in RSpec 4 as we will remove the monkey patch versions at that time. You should at least use RSpec.<method> for top level methods.

If there is something else that we should be using to achieve equivalent results I am happy to investigate that, otherwise can you clarify if there is an action item that you are suggesting?

I'm saying at least use RSpec.<method> over <method> as the monkey patched main methods are definietly not designed fo this. You could also experiment with RSpec::Core::ExampleGroup.new rather than the DSL and register them yourself, but some of that will be private api. I'm open to adding something to support that if appropriate.

That change is easy enough to make, but can you clarify whether you are saying there is a difference between

RSpec.describe do
# ...
end

and

describe do
# ...
end

?
Another issue I ran into is I extracted a helper method to deal with requirements here: https://github.com/p-mongo/mongo-ruby-driver/blob/1792/spec/support/crud.rb#L187
This method defines a context, and this context is not showing up in my formatter output. I tried RSpec.context but got an error about that only being allowed at top level. Can I define contexts in separate methods and have them be included in RSpec output?

@p-mongo the difference is the first is using an rspec method on an rspec module, and is the preferred way to create specs. The second is a legacy mixin of RSpec into Ruby's main object which is only available when rspec mixes it in during a defined setup process. It is going away in RSpec 4.
Secondly you need to ensure that context is invoked within an existing RSpec.describe block.
All of this is ruby class semantics, the DSL you are mis-using is creating classes to invoke. Consider that in your usage.
Honestly I would prefer you instantiate ExampleGroup and Examples directly. If you find problems with those I'll happily fix issues to make those public APIs.

I did this: https://github.com/p-mongo/tests/blob/master/rspec-id/spec/context_fn_spec.rb
The output is on the bottom. The "nested fn with yield" describe produces the same output as "nested fn" describe, but there is an additional context "my yielding context" defined for the "nested fn with yield" case.
I looked at https://github.com/rspec/rspec-core/blob/master/lib/rspec/core/dsl.rb to figure out what the DSL does and how to do it via ExampleGroup/Example but, sorry, I don't understand how to figure this out short of becoming an RSpec internals expert first.

I did this: https://github.com/p-mongo/tests/blob/master/rspec-id/spec/context_fn_spec.rb

The output is on the bottom. The "nested fn with yield" describe produces the same output as "nested fn" describe, but there is an additional context "my yielding context" defined for the "nested fn with yield" case.

If you look at the scoping during your blocks you'll see that the nested yield is resolving to the describe block, so the extra context is empty and skipped. Easily testable by adding an it directly between lines 26-27. (I checked the scope by using a simple puts self).

What classes do describe and context map to?
ExampleGroup

May I suggest if you don't want to look at RSpec internals you use shared_examples to declare your examples rather than programmatically creating things?

Thanks, using instance_exec instead of yield produces the desired results.
Is there any sort of documentation or discussion available for the motivation behind the changes mentioned here for rspec 4? So far my impression is rspec 4 is going to remove functionality that exists in rspec 3 but I haven't seen the motivation for the change, or a suggested path forward for users relying on the functionality that is potentially getting removed.

Is there any sort of documentation or discussion available for the motivation behind the changes mentioned here for rspec 4?

Sort of, this has been the plan since we re-wrote the API to be non monkey patching. The aim was always to eliminate monkey patching.

So far my impression is rspec 4 is going to remove functionality that exists in rspec 3

No functionality is being removed, a legacy API is being removed. This is the point of major version changes.

but I haven't seen the motivation for the change.

The motivation for the change is to completely eliminate monkey patching.

or a suggested path forward for users relying on the functionality that is potentially getting removed.


Feature/add searchapi web browsing

Pull Request Type

[x] ✨ feat
[ ] 🐛 fix
[ ] ♻ refactor
[ ] 💄 style
[ ] 🔨 chore
[ ] 📝 docs

Relevant Issues
closes #2223
AnythingLLM docs
What is in this change?

Added SearchApi to web browsing.
Added AGENT_SEARCHAPI_API_KEY in example environments variable files.
Added AGENT_SEARCHAPI_ENGINE that is responsilbe for choosing different search engines (Google, Bing, Baidu, Google News, etc.)

Additional Information
The end user can change the search engine used on the SearchApi side via the AGENT_SEARCHAPI_ENGINE parameter. Organic results share the same structure across all existing and upcoming SERP engines, so we will always have a title, snippet, and link. Google, Bing, Baidu, Google News, Bing News, Google Videos, Google Scholar, and Google Patents are already supported. Yandex and Yahoo are coming soon as well.
SearchApi is committed to supporting open-source integrations. Please feel free to mention @SebastjanPrachovskij for any future PR reviews related to SearchApi. We would be glad to help.

Screenshots:

Developer Validations

[x] I ran yarn lint from the root of the repo & committed changes
[x] Relevant documentation has been updated
[x] I have tested my code functionality
[x] Docker build succeeds locally

@SebastjanPrachovskij Excellent work. Made some small UI and changes specifically with the selection of a valid engine since I'm confident someone might misspell or use the wrong case in a free-form text input.


add datetime toordinal format extension method

Thanks for your pull request.
I try to keep the library as universally usable as possible.
Since your proposed extension produces culture specific output without taking the current culture into account I have to decline it.
Also, I would recommend you to use this code: https://stackoverflow.com/a/9130114
It removes the unnecessary heap allocation (from the int array), replaces the IColelction.Contains with a much faster switch-case and is easier to read.


Disappear the content of a file when using "> news.txt"

Prerequisites

[ ] Put an X between the brackets on this line if you have done all of the following:

Reproduced the problem in Safe Mode: http://flight-manual.atom.io/hacking-atom/sections/debugging/#using-safe-mode
Followed all applicable steps in the debugging guide: http://flight-manual.atom.io/hacking-atom/sections/debugging/
Checked the FAQs on the message board for common solutions: https://discuss.atom.io/c/faq
Checked that your issue isn't already filed: https://github.com/issues?utf8=✓&q=is%3Aissue+user%3Aatom
Checked that there is not already an Atom package that provides the described functionality: https://atom.io/packages

Description
Disappear the content of a file
Steps to Reproduce

In a console/terminal session I make an empty file
> news.txt

I open this file in atom. I put some text in it. And save the file.

I go back do my terminal and push enter

In atom all my text is disappeared.

Expected behavior: [What you expect to happen]
Nothing
Actual behavior: [What actually happens]
All the text in atom is gone/disappear
Reproduces how often: [What percentage of the time does it reproduce?]
100%
Versions
I am using OS X EL Capitan Version 10.11.6
$ atom --version
Atom    : 1.23.1
Electron: 1.6.15
Chrome  : 56.0.2924.87
Node    : 7.4.0
Additional Information
Any additional information, configuration or data that might be necessary to reproduce the issue.
I'am using atom on a Mac. I do not know what the behavior is on Windows, Linux, ....
When using "touch news.txt" everything work fine, only with ">  news.txt" is a problem.

This is the intended behavior. When the file is saved the contents in memory match the contents on permanent storage. Until the contents in memory are changed again (thereby making the in memory version "dirty"), Atom keeps the version in memory in sync with the version on permanent storage. So when you change the contents on disk by pressing Enter, Atom updates the copy in memory.
Since this is the expected behavior. I'm going to close this. Let me know if I've misunderstood anything.

Hello Lee-dohm,
I send you a small movie ..
Regards,
Gerrit
atom_strange.m4v.gz

That definitely is strange behavior. I can reproduce it on my machine using only /bin/bash. I don't know if this is a bug or just a strange interaction between bash, a possibly open file handle, and Atom though. Do you happen to know what bash is doing behind the scenes when you execute > filename? Because if it was exactly the same behavior as touch filename then Atom wouldn't behave differently.

I tried doing the same but then with vi. On a Mac, I got the same problem but doing on Ubuntu in a Virtualbox it works correctly. So, probably, Mac oriented. So, some minor bug, nothing to care about and not a problem with atom.