fix: preserve scroll position and fixed the Saved title

Details

This PR will preserve the scroll position and fixed the Saved title on large screen devices saved filter sections.

Fixed Issues

$ https://github.com/Expensify/App/issues/49218
PROPOSAL: https://github.com/Expensify/App/issues/49218#issuecomment-2351167485
Tests

1. Large screen device:
Precondition:
There is a long list of saved search until the saved search list is scrollable.

Go to app.
Go to Search.
Scroll the saved search list to the bottom.
Verify that the "Saved" title should remain on top when the list is scrolled down.
Select a search from the bottom of the list.
Verify that the list will not scroll up after selecting the search at the bottom of the list.

2. Small screen device:
Precondition:
There is a long list of saved search until the saved search list is scrollable.

Go to app.
Go to Search.
Tap on the Filter dropdown, the Filter modal will be opened.
Scroll the saved search list to the bottom.
Verify that the "Saved" title works as it is before because this change just applies to large devices.
Select a search from the bottom of the list.
Verify the Filter modal is closed properly.

[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).

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

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb 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 either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
[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 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(theme.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 the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:

[x] I verified that all the inputs inside a form are aligned with each other.
[x] I added Design label and/or tagged @Expensify/design so the design team can review the changes.

[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.

Screenshots/Videos

Android: Native

https://github.com/user-attachments/assets/152cdb71-3667-4309-bb5d-217438442d13

Android: mWeb Chrome

https://github.com/user-attachments/assets/f44ad04c-2086-4516-9dd5-3be1a4e125ab

iOS: Native

https://github.com/user-attachments/assets/37dca0ec-1ce4-40f5-b416-9c279605ab7e

iOS: mWeb Safari

https://github.com/user-attachments/assets/fc0d55a9-1905-41bf-82f7-05696e7c3ed9

MacOS: Chrome / Safari

https://github.com/user-attachments/assets/34c978d6-4d84-4904-923f-69be461f90ad

MacOS: Desktop

https://github.com/user-attachments/assets/534c78a4-a584-4b82-93e6-8973ec46f486

Reviewer Checklist

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

[ ] I verified the steps for local testing are in the Tests section
[ ] I verified the steps for Staging and/or Production testing are in the QA steps section
[ ] 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)
[ ] 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)

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

[ ] Android: Native
[ ] Android: mWeb Chrome
[ ] iOS: Native
[ ] iOS: mWeb Safari
[ ] MacOS: Chrome / Safari
[ ] MacOS: Desktop

[ ] 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
[ ] I verified proper code patterns were followed (see Reviewing the code)

[ ] 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).
[ ] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[ ] I verified that comments were added to code that is not self explanatory
[ ] 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.
[ ] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
[ ] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[ ] 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 either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
[ ] 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.
[ ] I verified the JSDocs style guidelines (in STYLE.md) were followed

[ ] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[ ] I verified that this PR follows the guidelines as stated in the Review Guidelines
[ ] 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)
[ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[ ] 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
[ ] If a new component is created I verified that:

[ ] A similar component doesn't exist in the codebase
[ ] All props are defined accurately and each prop has a /** comment above it */
[ ] The file is named correctly
[ ] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[ ] The only data being stored in the state is data necessary for rendering and nothing else
[ ] 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)
[ ] 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)
[ ] All JSX used for rendering exists in the render method
[ ] 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

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

[ ] 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

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

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

[ ] 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.
[ ] 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)
[ ] 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.
[ ] 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.
[ ] If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:

[ ] I verified that all the inputs inside a form are aligned with each other.
[ ] I added Design label and/or tagged @Expensify/design so the design team can review the changes.

[ ] 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.
[ ] 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.
[ ] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

: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.

🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.0.35-7 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

🚀 Deployed to production by https://github.com/grgia in version: 9.0.35-7 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅


Update app.py

Adding some comments where felt necessary.

This is great, thank you for taking time to help improve the code.
For the sake of clarity, could you move this to above the code instead of to the side? Just thinking about line wrapping.
It would also be great if you could replicate it to the other samples as well. If not, I can try to do it after //Build finishes

Of course, I would be glad to do that. I will do with other samples too. :-) :-)

Closing as you've made the requested changes in another PR


Support local images

I have been trying to render local images but I was not able to do it, is there any way to do it?

Me too.
I also need support. I am having this error when taking a picture (https://user-images.githubusercontent.com/15552410/71166953-511c6980-2286-11ea-994e-553541863859.png)

Hi there!
This issue related #16

I have been trying to render local images but I was not able to do it, is there any way to do it?

I solved it by wrapping App.tsx in

//


Add support to configure base/content paths

Hi,
For the vapor 3 AdminPanel I wanted to disable the Sidebar in the admin panel for some user roles, but currently I would need to change the base/content leafs and all leafs that are pointing at the base leaf, so it could be nice to have way of configuring the base/content path at init.
I don't have time to do it myself right now, so I am submitting this issue so I can have a look later or so somebody else can help me add this feature 😊

Issue is no longer relevant as such solutions have already been implemented across several projects. Using the configurable parameter sidebarMenuPathGenerator we can customize the logic that generates references to different nav-templates based on user roles


White line between tabs

Code
const TabsNavigator = TabNavigator({
  Location:   { screen: Location },
  Coupon:     { screen: Coupon },
  Map:        { screen: Map },
  Hours:   { screen: Hours },
  Contact:    { screen: Contact },
}, {
  tabBarComponent: props => <TabBarBottom {...props} />,
  tabBarPosition: 'top',
  animationEnabled: false,
  swipeEnabled: false,
  tabBarOptions: {
    inactiveBackgroundColor: '#303033',
    activeBackgroundColor:  '#303033',
    showLabel: false,
    style: {
      height: 40,
      borderTopWidth: 0,
      backgroundColor:  '#303033',
    }
  },
});

Current Behavior
On large screens (iPad Mini, iPhone 6 Plus) I get a thin white line between two tabs.
Expected Behavior
No white line.
Your Environment

software
version

react-navigation
1.0.0-beta.13

react-native
0.50.3

Thanks for reporting, but it looks like you are using a really old version of the library.
Can you please update to latest and let me know if it still happens?

I've got it with 1.0.0-beta.21 as well, it looks like it appears when the tabs are an uneven number. When I change my tabs to 4 it's gone

Probably a bug with React Native and subpixel positioning.
Can you change inactiveBackgroundColor and activeBackgroundColor to transparent and see if it still happens?

closing because inactive and better fit for https://github.com/react-native-community/react-native-tab-view

@satya164 I had the same issue, and your method worked. Thanks.


Create mdbook documentation

Do you think we need adjust the structure of book? @nyurik

Do you think we need adjust the structure of book? @nyurik

Sure, any restructuring is possible - but I think we should start with matching the current readme, migrate to it (e.g. make it auto-publish to the maplibre.org/martin site), and then start changing - otherwise we will have two duplicate places to keep up to date, and that's never good.

Yeah.One PR for one thing


Restore tests for DirectoryProvider and assosciated pre-processor

Continuation of fix for #305, requirement for blueapi 376 (moving the DirectoryProvider from blueapi)
Instructions to reviewer on how to test:

Run multiple scans with the FakeDetector or a StandardDetector implementation
Confirm h5 files do not get overwritten

Checks for reviewer

[ ] Would the PR title make sense to a scientist on a set of release notes
[ ] If a new device has been added does it follow the standards

Is this contingent on a blueapi PR?

This is a soft requirement for the blueapi PR, the blueapi PR removes these tests but they were missed when moving the DirectoryProvider into dodal.


Unset parameter expansions

Consider having —
% unset foo
% echo $foo

— highlight the expansion $foo as an error/warning, since the parameter is unset.

Cross-referencing #691.

Closing in favour of #509.

Reopened since #718 fixes this specifically, but not all of #509.

Closing in favour of #450.


Add unit test helpers for new json serde

fixes: https://github.com/creek-service/creek-kafka/issues/483
This involved making the JsonSchemaStoreClient part of the API, which mean introducing our own Schema type (to avoid exposing none-Creek types on API interfaces).
Also in this change, I've added testBuilder() methods to the other options types to match the one on the JsonSerdeExtensionOptions type.
Also added MockXXX interface types that extend injectable types, making it easier for others to implement and less likely that adding new methods causes compilation issues.

coverage: 97.196% (-0.2%) from 97.347%
when pulling bac2205fe0e260b174c048ef05cdb36e10c09e8a on unit-test
into 37b1be4c3615c50739afa03795935f92eeef112c on main.


Change default get_function_annotation implementation

See https://github.com/STEllAR-GROUP/hpx/pull/5694.
This needs more work to actually implement the suggestions in https://github.com/STEllAR-GROUP/hpx/pull/5694.

Performance test reportHPXLocal PerformanceComparisonBENCHMARKNO-EXECUTORFuture Overhead - Create Thread Hierarchical - Latch(=)InfoPropertyBeforeAfterHPXLocal Commitf499a2233385060b8a2612ab88163e62b0881888cffcbafce438d3bd253b3bf3053c2fbb5c45881fHPXLocal Datetime2021-12-10T13:50:04+00:002021-12-20T09:40:45+00:00Datetime2021-12-10T15:19:42.442217+01:002021-12-20T10:50:22.521517+01:00Compiler/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0EnvfileClusternamedaintdaintHostnamenid00243nid00407ComparisonBENCHMARKFORK_JOIN_EXECUTORPARALLEL_EXECUTORSCHEDULER_EXECUTORStream Benchmark - Add(=)(=)(=)Stream Benchmark - Scale(=)(=)(=)Stream Benchmark - Triad(=)-(=)Stream Benchmark - Copy(=)(=)(=)InfoPropertyBeforeAfterHPXLocal Commitf64fbd02165a132a6276cedd14c586910abb79e4cffcbafce438d3bd253b3bf3053c2fbb5c45881fHPXLocal Datetime2021-11-12T11:29:27+00:002021-12-20T09:40:45+00:00Datetime2021-11-12T12:57:50.824026+01:002021-12-20T10:50:37.117794+01:00Compiler/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0/apps/daint/SSL/HPX/packages/llvm-11.0.0/bin/clang++ 11.0.0EnvfileClusternamedaintdaintHostnamenid00007nid00407Explanation of SymbolsSymbolMEANING=No performance change (confidence interval within ±1%)(=)Probably no performance change (confidence interval within ±2%)(+)/(-)Very small performance improvement/degradation (≤1%)+/-Small performance improvement/degradation (≤5%)++/--Large performance improvement/degradation (≤10%)+++/---Very large performance improvement/degradation (>10%)?Probably no change, but quite large uncertainty (confidence interval with ±5%)??Unclear result, very large uncertainty (±10%)???Something unexpected…


Use python's re module instead of pyformlang to build regex ASTs

Removes dependency on pyformlang, instead using python's re module directly.
This ensures that we conform as tightly to python's regex semantics as possible, and it should be easy to raise good exceptions rather than failing silently if we don't support a given corner of regex (e.g. lookarounds).
In python < 3.11, the relevant imports come from sre_parse and sre_constants, while in 3.11 and greater, we have to import from re._parser and re._constants. I'm a little bit concerned about using what's not considered public API, but it seems that there has been good effort to keep this part of re relatively stable even into 3.13. I've noticed some other libraries, e.g. lark reaching into the re/sre modules in the same way.

Codecov Report
Attention: Patch coverage is 89.15663% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 58.08%. Comparing base (c9e71fb) to head (076f377).

Files
Patch %
Lines

guidance/library/_regex.py
89.15%
9 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #854      +/-   ##
==========================================
+ Coverage   56.94%   58.08%   +1.13%
==========================================
  Files          63       63
  Lines        4571     4571
==========================================
+ Hits         2603     2655      +52
+ Misses       1968     1916      -52

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

@riedgar-ms starting to feel pretty happy with this. I'd appreciate it if you could give it a once-over when you have the time :)

I still need to take a closer look at what's causing some models to fail some of the tests. I do seeing one interesting failure so far though. Here's a test that's failing for phi2:

     def test_pattern_kleene(selected_model):
        lm = selected_model
        lm += "The Lord is my"
        x = lm + gen(name="tmp", max_tokens=10)
        y = lm + gen(name="tmp", regex=".*", max_tokens=10)
>       assert y["tmp"].startswith(
            x["tmp"]
        )  # TODO: we just check startswith because exact token limits are not perfect yet...

E       AssertionError: assert False
E        +  where False = <built-in method startswith of str object at 0x7f4398106830>(' shepherd, I shall not want.\nThe Lord')
E        +    where <built-in method startswith of str object at 0x7f4398106830> = ' shepherd, I shall not want.'.startswith

I don't think I agree with the test actually -- it implies that .* should be less restrictive than gen without any regex.
Depending on what re flags, we're imagining are being used (side note: should we support flags? :thinking:), .* should match anything without a newline. The test fails in this case because the unrestricted gen includes a newline.

@Harsha-Nori pinging for visibility

Thanks Hudson! I'll

I still need to take a closer look at what's causing some models to fail some of the tests. I do seeing one interesting failure so far though. Here's a test that's failing for phi2:
     def test_pattern_kleene(selected_model):
        lm = selected_model
        lm += "The Lord is my"
        x = lm + gen(name="tmp", max_tokens=10)
        y = lm + gen(name="tmp", regex=".*", max_tokens=10)
>       assert y["tmp"].startswith(
            x["tmp"]
        )  # TODO: we just check startswith because exact token limits are not perfect yet...

E       AssertionError: assert False
E        +  where False = <built-in method startswith of str object at 0x7f4398106830>(' shepherd, I shall not want.\nThe Lord')
E        +    where <built-in method startswith of str object at 0x7f4398106830> = ' shepherd, I shall not want.'.startswith

I don't think I agree with the test actually -- it implies that .* should be less restrictive than gen without any regex. Depending on what re flags we're imagining are being used (side note: should we support flags? 🤔), .* should match anything without a newline. The test fails in this case because the unrestricted gen includes a newline.

Great catch! I agree that this is a test misspecification :). Perhaps we can change the test to use a more restrictive kleene star?

I like the direction this PR is moving in general, standardizing on the python re module would be nice to align with user expectations. Happy to do a more formal code review when you think it'd be helpful :). One dimension we should consider measuring, which isn't captured in our tests, is runtime performance (I'd expect re to be at least as good as pyformlang)?

I like the direction this PR is moving in general, standardizing on the python re module would be nice to align with user expectations. Happy to do a more formal code review when you think it'd be helpful :). One dimension we should consider measuring, which isn't captured in our tests, is runtime performance (I'd expect re to be at least as good as pyformlang)?

I agree re: aligning with user expectations!
Let me know when you have some time for a review -- it would definitely be helpful.
Re: testing performance -- interesting question, and definitely worth exploring. My expectation is also that re should be at least as good as pyformlang, and that's certainly the case for some special cases (e.g. large quantifier ranges do something scary and recursive in pyformlang, while this implementation just adds a bunch of optional grammars).

@riedgar-ms I refactored the code used for testing json generation a bit by moving _generate_and_check over to utils. It's a pretty reusable piece of testing machinery which asserts that a Mock model can actually generate the string. I also added another check to that util to make sure that grammar.match correctly consumes the string (and appropriately captures).
Oddly, I'm running into some problems now around getting the mock model to actually STOP when it should in the regex tests, even with the chr(7) stop character in there... You hit this problem before -- any ideas?

I think there is a bug associated with wrapping a grammar in with_temperature more than once (the innermost one seems to be what's respected)

Perhaps not the perfect implementation, but this is intentionally by design. We were thinking about someday having an order of precedence, with inner specifications taking priority, so you could do things like (made up syntax):

lm = guidance.Model("blah", temperature=0.7) # overall default

lm += gen('hi') # uses 0.7

with Sampler(temperature=0.5):
    lm += gen('hi2') # uses 0.5

    lm += with_temperature(gen('hi3'), temperature=0.3) # uses 0.3

    lm += gen('hi4') # uses 0.5

lm += gen('hi5') # uses 0.7

The idea is that we could let people use a variety of tools for setting sampling parameters that apply to different ranges of their generation/grammar. None of this is implemented, however, except how temperature is represented in the grammar tree

Now that I think about it more though, it's true that overriding a function which sets a default sampling parameter is not ergonomic (i.e. we must pass down the kwarg, not just wrap it with with_temperature as users might expect). Worth brainstorming on this in a separate issue if you'd like to open one up

Ah, makes sense. I'll look at where it's being set inside of gen to see if I can't just make it set once and only once in the call to regex.
Speaking of which, how careful do you think we need to be about only making backwrds-compatible changes to the API? For now, I will add name and  temperature to regex as keyword-only, but just curious for future reference.
For regex in particular, it seems that it's more commonly accessed indirectly through gen, but I'm not sure what people are doing in the wild.

We'll want to be more stringent about back-compat in the near future, but I'm ok with the idea of API changes now. My preference is to accumulate a series of changes we'd like to make, then do a single update w/ appropriate announcement vs. micro adjustments per-release

Now that I think about it more though, it's true that overriding a function which sets a default sampling parameter is not ergonomic (i.e. we must pass down the kwarg, not just wrap it with with_temperature as users might expect). Worth brainstorming on this in a separate issue if you'd like to open one up

Just a thought, but it might be sensible to make the default be None, where we don't actually wrap in wih_temperature if it's unset?

For regex in particular, it seems that it's more commonly accessed indirectly through gen, but I'm not sure what people are doing in the wild.

It's mostly accessed through gen today, which is good ergonomics for this heavy hitting use case, but perhaps not very uniform/general (for example, should we also expose json and others through gen directly)? In general I like having kwargs on a smaller number of functions vs. forcing users to learn a broader function space, but it's worth thinking carefully about the design here going forward too

Now that I think about it more though, it's true that overriding a function which sets a default sampling parameter is not ergonomic (i.e. we must pass down the kwarg, not just wrap it with with_temperature as users might expect). Worth brainstorming on this in a separate issue if you'd like to open one up

Just a thought, but it might be sensible to make the default be None, where we don't actually wrap in wih_temperature if it's unset?

Yes, I think this pattern generally works, I was thinking more about if/when we write functions that are perhaps opinionated about a temperature (I can see this being common with temp=0 for some things). But I think there's options here, we could write those to check for None internally maybe, or have None be the default on with_temperature style functions (which does a no-op and doesn't change anything), etc.


AZURE_OPENAI_ENDPOINT env var conflicts with OPENAI_API_BASE, while OPENAI_API_BASE is being used in MLFlow to point to Azure OpenAI endpoint.

Checked other resources

[X] I added a very descriptive title to this issue.
[X] I searched the LangChain documentation with the integrated search.
[X] I used the GitHub search to find a similar question and didn't find it.
[X] I am sure that this is a bug in LangChain rather than my code.
[X] The bug is not resolved by updating to the latest stable version of LangChain (or the specific integration package).

Example Code
Environment variables:
# for langchain
OPENAI_API_VERSION='...'
AZURE_OPENAI_API_KEY='my-key'
AZURE_OPENAI_ENDPOINT='my-azure-openai-endpoint'

# for mlflow -- this conflicts
OPENAI_API_BASE='my-azure-openai-endpoint'

Code to cause the error:
from langchain_openai import AzureOpenAIEmbeddings, AzureChatOpenAI

model = AzureChatOpenAI(
    azure_deployment="my-deployment-name",
)

# or
model = AzureOpenAIEmbeddings(
    azure_deployment="my-deployment-name",
)

Error Message and Stack Trace (if applicable)
ValidationError: 1 validation error for AzureChatOpenAI
__root__
  As of openai>=1.0.0, Azure endpoints should be specified via the `azure_endpoint` param not `openai_api_base` (or alias `base_url`). (type=value_error)

Description
AzureOpenAIEmbeddings and AzureChatOpenAI automatically pick up the env vars and throwing errors complaining about having both azure_endpoint param and openai_api_base.
OPENAI_API_BASE env var is being used in other packages, like MLFlow.
System Info
#pip freeze | grep 'langchain\|mlflow'
langchain==0.1.20
langchain-community==0.0.38
langchain-core==0.1.52
langchain-openai==0.1.6
langchain-text-splitters==0.0.1
mlflow==2.12.2

Have you tried specifying everything when you create the model?
For example:
model = AzureChatOpenAI(
            openai_api_version="2023-05-15",
            azure_deployment=os.getenv("AZURE_OPENAI_DEPLOYMENT_NAME"),
            temperature=0,
            azure_endpoint=os.getenv("AZURE_OPENAI_ENDPOINT"),
            api_key=os.getenv("AZURE_OPENAI_API_KEY"),
        )

Not sure if this would work but give it a try.

I have the exact same problem as OP, and no, that does not work.
import os
from langchain_openai import AzureOpenAIEmbeddings

from dotenv import load_dotenv
from pathlib import Path

load_dotenv('azure.env')

embeddings = AzureOpenAIEmbeddings(
    azure_endpoint=os.environ["AZURE_OPENAI_ENDPOINT"],
    api_key=os.environ["AZURE_OPENAI_API_KEY"],
    azure_deployment=os.environ["AZURE_OPENAI_EMBEDDINGS"])

text = "Hello, world!"
query_result = embeddings.embed_query(text)
print(query_result)

In azure.env, if I don't set OPEN_API_BASE at all, the code works. If I do (even with the same value as the AZURE_OPENAPI_ENDPOINT), I get the same exact error that OP sees. So no, adding the endpoint to the arguments does not fix this problem.

Just submitted a PR with a fix


Constant warning in logs to update API version to 1.2

I am experiencing a huge increase in timeouts on the UPS’s I am connecting to and a warning that the plugin should be using API version 1.2

Timeouts are likely caused by issues with your network and or with the systems running upsd.  I'd expect heartbeat drift warnings if the system running Homebridge UPS is under stress.  If upsd runs on a Raspberry Pi, double check that the swap file isn't fully used - Pi OS comes to a grinding halt when that's the case.
The API version warning is issued only when Homebridge UPS connects to upsd, usually only on startup.  If you see that more often, the connection likely gets lost, after which Homebridge UPS reconnects.
Run Homebridge in DEBUG mode and set Log Level on the UPS accessory to 2 to make Homebridge UPS issue debug messages on each request sent to and response received fromupsd.


add custom interval to scrape metrics

Is this a new chart
No
What this PR does / why we need it:
This PR lets us set the interval on nri-kubernetes-definition.yml which can help customize the frequency of the datapoints that are sent to Newrelic.
Checklist
[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

[x] Chart Version bumped
[x] Variables are documented in the README.md
[x] Title of the PR starts with chart name (e.g. [mychartname])

 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.

Hi @adimania  thanks for your contribution , this is related to a FR described on #344 .
We are thinking to follow a different approach for this described here that would cause a breaking change if we merge your proposed solution here.
So event if this is not a breaking change now it would make that our proposed solution to this be breaking change.

Hi @gsanchezgavier. That is awesome. I am happy to see that work is being done in this direction.
Do you have an ETA for this feature? I would be happy to close this PR in favour of community's decision to move the files in /etc.

not sure about ETA this still need to be prioritized.

Hi @adimania
Thanks for your contribution, we're planning to implement this feature on FY22 Q2.
I will updated you once the feature is released.
Regards

Closing this due to the discussion above


Support using secrets instead of configmap

If configmaps contains secret data, we need to use secrets.

@sadlil . example HAProxy config as stats secrets.


[region-isolation] Fix handling of coroutine apply results.

In this part of the code, we are attempting to merge all of the operands into
the same region and then assigning all non-Sendable results of the function to
that same region. The problem that was occuring here was a thinko due to the
control flow of the code here not separating nicely the case of whether or not
we had operands or not. Previously this did not matter, since we just used the
first result in such a case... but since we changed to assign to the first
operand element in some cases, it matters now. To fix this, I split the confused
logic into two different easy to follow control paths... one if we have operands
and one where we do not have an operand. In the case where we have a first
operand, we merge our elements into its region. If we do not have any operands,
then we just perform one large region assign fresh.
This was not exposed by code that used non-coroutines since in SIL only
coroutines today have multiple results.
rdar://132767643

@swift-ci smoke test


Update KYVE info and logo

Thanks. Replaced with this (which retains a 'chain' logo)
https://github.com/cosmos/chain-registry/pull/2631


JavaErrorParser uses i18n dependent prefix

https://github.com/sbt/zinc/blob/b55f23c60503538ab4c7c9dae515b751292d6e68/internal/zinc-compile-core/src/main/scala/sbt/internal/inc/javac/JavaErrorParser.scala#L52-L54
As result JavaErrorParser only works when Javac/Javadoc output is in English or Chinese/Japanese.

No idea how to access the severity of the issue. There might be no one hitting the issue in practice, or they simple are not reporting on Github as Github's default language is English.


Docker for opengrid/release

Hi all,
I tried to get the notebooks inside the docker container by default.  I'm almost there, except that a tar command in my Dockerfile is not doing anything.  When I run the command manually inside /bin/bash in the container, it works well and the files are in the right place.
I'm afraid I need a little help, maybe @wouterh can figure out what's wrong?
Thanks,
Roel

Coverage decreased (-0.3%) to 71.678% when pulling b9cafb96bb5aea14e55c3f7e32a80925f7200949 on saroele:develop into d8fbf8c2f9aa77f586c4b9452fc88ec154d3376d on opengridcc:develop.

Ha, sometimes a lunch break is what you need.
I simplified the entire Dockerfile by adding the notebooks directly in the build.

Coverage decreased (-0.3%) to 71.678% when pulling 21bc57acaa33791ce0450551dd791bd5952df428 on saroele:develop into d8fbf8c2f9aa77f586c4b9452fc88ec154d3376d on opengridcc:develop.

Coverage decreased (-0.3%) to 71.678% when pulling c17ff2512ddb4f089dacbd66c5cf9620dfd0624f on saroele:develop into d8fbf8c2f9aa77f586c4b9452fc88ec154d3376d on opengridcc:develop.

I have finalised the docker image and run scripts for the development environment.  I think we should leave the release version unfinished for now. Everyone who will use this image is a developer now, and will first clone the github repo to get the run-docker-dev.sh script and start the docker.
One thing we still have to fix: where to put the data (.tmpo database, caches, ...) when using the docker image?  I think the best way is to mount a second volume inside the docker with the data so it can get stored on the host.  But, to find which folder you want to mount inside the docker, you would have to read the opengrid.cfg file.  This is feasible, but will make the run-docker-dev.sh script a little bit heavier.
An other solution is to impose that the data folder is inside the opengrid root (where the docker-run-dev.sh lives).  This is the folder that gets mounted, so we can access a /data folder there without an additional mount.  By adding /data to the .gitignore, we can avoid that it gets pushed.  Maybe this is currently the most elegant option: restricts the freedom where to put the data but is uniform for anyone and easy.
Any other thoughts?

Coverage decreased (-0.3%) to 71.678% when pulling adcfa52261e23c07dea2bbf96593dfbd91aef794 on saroele:develop into d8fbf8c2f9aa77f586c4b9452fc88ec154d3376d on opengridcc:develop.

The path that is in the opengrid.cfg is the path within the docker container, so I would put /data in there as the path and declare that as a volume in the Dockerfile. A developer can then choose where the data is stored by choosing how the volume in /data is used (see https://docs.docker.com/engine/userguide/containers/dockervolumes/)

@wouterh good idea, I have adapted both the release and the dev Dockerfiles.
What about the docker-run-dev.sh and docker-run-release.sh scripts.  Will we mount a default folder ($PWD ?) to the /data volume.  Developers who want another folder to be mounted to /data will then have to adapt their shell script or use the appropriate docker run command directly in their terminal?

Current version of the pull request will mount the ./data folder to the /data folder in the container.
I also updated the instructions on the docker hub: https://hub.docker.com/u/opengrid/dashboard/

Coverage decreased (-0.3%) to 71.678% when pulling 70930b44c5490e357082df157c986084a78b755a on saroele:develop into d8fbf8c2f9aa77f586c4b9452fc88ec154d3376d on opengridcc:develop.

What about an environment variable? The script can check if it is set and otherwise use a default.


DnssdManager never stops advertising

Basically what the comment says here: the registration remains discoverable, even if the app is not running.
https://github.com/Microsoft/Windows-appsample-networkhelper/blob/master/NetworkHelper/DnssdManager.cs#L114
It seems DnssdServiceInstance should have some "unregister" method

Thanks for reaching out, jvcleave! I have forwarded your request to the engineering team that is responsible for the DnssdServiceInstance class for further investigation. In the meantime, you can use the UdpManager.cs and UdpParticipant.cs classes instead, by replacing any instances DnssdManager/DnssdParticipant accordingly—These classes make use of UDP multicast for discovery instead of DNS-SD. Don’t hesitate to open another issue if you run into other problems.

any updates on this?

jvcleave, there are no updates as of now. I just reached out to the team and will get back to you when I hear something. May I ask what you are trying to do so that I can potentially suggest other alternatives in the meantime?

I have a Hololens app that is running a StreamSocket service that I want to advertise so another UWP can connect to it and send it commands.
As it stands currently the Hololens app is advertising the DNSSD service and the remote app is able to connect. However every time the Hololens app starts up (which can be often) it pollutes the network with another DNSSD service (registering a the same instance name causes an error).
The remote UWP app also has to sort through all of the service instances (I have them timestamped in TextAttributes) in order to find the most recent one.

jvcleave, does the Hololens app call Dispose() on the stream socket listener before it exits? DNS-SD de-registration is supposed to be tied to the lifetime of the socket. If yes, you may want to consider using UDP multicast for discovery while the team is investigating this. I'll keep you posted here when I hear something from them.


Garden Tools Backend

Added graphics for a shovel and watering can
Added tools to the GardenScreen page
Pressing a tool will set it as the "selectedItem" - when a tool is selected, it can be used to water plants or dig up plants
Watering can successfully waters a plant
Shovel digs up a plant and adds it back to the inventory
Added functions for initializing user fields when an account is created through firebase - however, these functions are not implemented. Future issue must be created to add firebase integration to garden plots and inventory.

Lgtm


Add a new Pushover provider

Summary
I am not sure if I missed something. There are a lot of files where providers are referenced, but I think I got all of them.

Adds new provider named Pushover with corresponding tests
Adds Pushover as a provider to the configuration and adjusts test accordingly
Adds Pushover to alerting_test.go, provider.go and type.go
Updates the readme with configuration details

Before merging: what would be the best way to properly test this with real API calls? Is there any process defined or should I just try to build it and check if a config works?
Additional thoughts/ideas that could be implemented later:

Add support for other configuration options, like device support, the Gatus URL in it or setting a sound
Allow overriding of title, priority,... per group like the PagerDuty provider does.
Allow usage of placeholders in title and description like the custom provider does.

Related: ##129
Checklist

[x] Tested and/or added tests to validate that the changes work as intended, if applicable.
[x] Added the documentation in README.md, if applicable.

Codecov Report
Base: 82.00% // Head: 82.05% // Increases project coverage by +0.05% :tada:

Coverage data is based on head (14a7e01) compared to base (90bb8f7).
Patch coverage: 86.66% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #405      +/-   ##
==========================================
+ Coverage   82.00%   82.05%   +0.05%
==========================================
  Files          55       56       +1
  Lines        4178     4225      +47
==========================================
+ Hits         3426     3467      +41
- Misses        586      590       +4
- Partials      166      168       +2

Impacted Files
Coverage Δ

alerting/provider/provider.go
100.00% <ø> (ø)

alerting/provider/pushover/pushover.go
86.36% <86.36%> (ø)

config/config.go
76.06% <100.00%> (+0.10%)
:arrow_up:

controller/handler/handler.go
79.41% <0.00%> (+1.28%)
:arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

I have built the Docker container with the latest changes and tested it with this config:
---
debug: true

alerting:
  pushover:
    application-token: ar5k...wkg4
    user-key: 2ZHm...cWed
    title: "Pushover Gatus Test"
    default-alert:
      enabled: true
      description: "Healthcheck failed for [ENDPOINT_URL]"
      failure-threshold: 1
      success-threshold: 1
      send-on-resolved: true

endpoints:
  - name: dashboard
    url: https://my-private-domain.com
    interval: 30s
    alerts:
      - type: pushover
    conditions:
      - "[STATUS] == 200"

Working as expected.
What I would like to add before merging this is to add support for placeholders in title and description like the custom provider does. I think this should be handled by a helper function so that there's no code duplication, because currently it's just hardcoded into the custom provider. Any thoughts an that?

@Kovah Support for placeholders will be handled separately.
See https://github.com/TwiN/gatus/issues/287

Before merging: what would be the best way to properly test this with real API calls? Is there any process defined or should I just try to build it and check if a config works?

Using the MockRoundTripper just like in PagerDuty was the right approach, and you've already done it.
There's a couple of integration tests that send real requests, but I try to avoid them as much as possible, and when it's unavoidable, I either use a website that I own or a stable public service (e.g. example.com, example.org, a public DNS, etc.), because they have predictable outputs or I can control the outputs myself.
As such, I wouldn't feel really comfortable having an integration test for something like pushover unless they explicitly provide permission.

@Kovah Excellent work!
Thank you for the contribution!

Great, thanks for merging!


Join a Community on Status Web

As it isn't clear this will be implemented, we move it to the backlog.


Opaque diagnostic message when a const constructor contains a callback

This snippet shows an opaque diagnostic message:
  const MyWidget(
    builder: (context, child) => Widget(),
  );

A value of type 'Null' can't be assigned to a parameter of type 'Widget Function(BuildContext, Widget)' in a const constructor.

cc: @bwilkerson
DartPad example: https://dartpad.dev/?id=8178b5031899610bfa75ef309845be47

I run into this more often than I used to because the default Flutter lints include the prefer_const_constructors rule

Possibly related to #46792


feat: Task 3: Cheap Analysis with Comparison of Lift Card Users’ Initial boardings with Low Income Populations by Census Block or Hexagons

This makes for some cheap analysis atm. This may make more sense when we actually use people’s home/work locations instead of initial boarding locations
One argument against calling this cheap analysis is that we don’t necessarily need to treat this analysis as one that uses initial boardings as a proxy for home/work locations. It is possible to simply plot the transit activity including transfers to see overall behavior. But in such a case, comparing transit activity with low income population by census block does not make sense.

Cancelling this task, as it has significant overlap with other tasks.


AssertionError

assert llm_source in ["gpt-4", "gpt-3.5-turbo", "claude"], f"llm_source must be one of [gpt4, gpt-3.5-turbo, claude], got {llm_source}"
AssertionError: llm_source must be one of [gpt4, gpt-3.5-turbo, claude], got gpt4
I didn't get claude api key, but I get gpt4, do I need to get both of them?thanks.

No you don't need both API keys. But the assertion error here is because of my typo in the README, it should be -llm gpt-4 and not -llm gpt4, fixing that now!


remove profile for fcrepo-connector-file, which is unmaintained

@ruebot: looks like the answer is no: https://wiki.duraspace.org/display/DSP/Contributor+License+Agreements
@bcail can you fill out an iCLA listed on that site and send it to<EMAIL_ADDRESS>

Here is a JIRA ticket for tracking: https://jira.duraspace.org/browse/FCREPO-2371

Just submitted my CLA.

@acoburn / @ruebot : @bcail's iCLA is now on record. Feel free to proceed.

Thanks @bcail!


Implement workaround for LeaseSpec serialization issue.

See #5 relating to the issue this attempts to workaround.
Rather than serialize a LeaseSpec which has a variable sized timestamp
formatting, this forces the timestamps to the six digits of subsecond precision
currently required by the k8s API.

Although it looks like to take the fix from k8s-openapi we might need to also bump kube and there's been breaking API changes from the version we're currently using.

I think the easiest way forward would be to update #40 to use this patch.

Let's land #40 first and then see where we are.

#40 is merged. I still think we should probably use this fix as I don't know when a new version of kube will be released and I don't think we should be using master just for this

I'm +1 on going forward with merging this. Would it make sense to add an additional comment under lease_spec_definition to remove this code once we update k8s-openapi? That way in case it gets lost we can always reference back to this and pull it out safely.

Wait - why not update k8s-openapi to the revision that specifically fixes our issue?

Oh! Is that release out now?

There isn't a release yet - but there is a fix in master that specifically solves our issue - https://github.com/Arnavion/k8s-openapi/commit/35cbddfeb26726bc1a0c882a40584c262fb53726

Okay. I'm of the opinion that I'd like to have something that works so we can move forward and test things out on master. Once the new crate version is available, we can update krustlet and remove this section of code.
Does that seem fair?

We could also pin to the master branch of k8s-openapi, but this seems a little less risky to introduce new issues.

Ok, then let's merge this now and update once the crate has a new release.
Thanks a lot, @peterhuene!


No expandable menu for GIT procedures in Navigation Bar

docs/home/procedures/git-procedures/set-up-wip-for-git.md is not a HTML file. Thus, the script, scripts/build_nav_data.py, does not consider this page for inclusion into the navigation bar.

Updated scripts/build_nav_data.py to accommodate markdown files.


great work.

i have been looking for flip book plugin, but what i get is only a legacy code which is not operates properly in react.  thank you :) you saved my ass

Thanks 😄


Add stride adaptor

Boo, CodeCov claims we're not testing the detail::advance(seq, cur, 0) cases, even though I'm pretty sure we are. Oh well.


[NG] new checkbox implementation

This removes the old clr-checkbox implementation for clrCheckbox to align with new forms.
It also replaces a few uses of old checkbox internally in tree view and datagrid.
Part of #1878

Does package-lock.json need to change or can we revert it?

The documentation still needs to be updated: https://vmware.github.io/clarity/documentation/v1.0/checkboxes

It hasn't been released yet, but will be updated.

@gnomeontherun do you know which release this will go in? We're waiting on checkboxes to be restored again.

@mcosta74 Sorry I might have misread this issue. I was talking about the checkboxes on forms, at the moment they are not implemented.

@farlock85 Checkboxes are part of 1.0.0-rc.1 and above, see https://vmware.github.io/clarity/documentation/v1.0/checkboxes.


[WIP] Replace deprecated / async-unsafe lifecycles

Note this PR is a work in progress and is not yet ready for review. CI failures are expected, because I'm pointed at a version of React that I've built locally.

Replace componentWillMount, componentWillReceiveProps, and componentWillUpdate with async-safe lifecycles in advance of React 16.4 deprecation warnings. Use react-lifecycles-compat to ensure backwards compatibility.
Relates to reactjs/rfcs/pull/6
In-progress reference documentation available at reactjs/reactjs.org/pull/587 and reactjs/reactjs.org/pull/596
RFC at https://github.com/reactjs/rfcs/blob/master/text/0006-static-lifecycle-methods.md
Resolves #1008

Also took the time to replace a few constructor-binded methods with arrow functions.

Just a quick update- the bulk of the migration is done, but Grid triggers a warning due to a setState call during render. I'm in the process of fixing this locally, but it has a bigger footprint than I wish because it involves a lot of instance properties/methods.

At this point, I find myself wanting to make everything in Grid static, except for the lifecycles that are required to be instance methods- but even those, I think should just defer to static methods.
My initial attempt to only update things that need access to cached values on the instance feels like it's getting out of control. So many of the helper methods call each other.
It's unfortunate that there's so much complexity within the timing and sequence of things Grid does.

Made some progress today but got distracted by another alpha release and some other work things. Will resume tomorrow!

@bvaughn I can help migrate to getDerivedStateFromProps :)

Codecov Report

Merging #1009 into master will increase coverage by 0.02%.
The diff coverage is 95.48%.

@@            Coverage Diff             @@
##           master    #1009      +/-   ##
==========================================
+ Coverage    90.2%   90.23%   +0.02%
==========================================
  Files          59       59
  Lines        1675     1669       -6
==========================================
- Hits         1511     1506       -5
+ Misses        164      163       -1

Impacted Files
Coverage Δ

source/WindowScroller/WindowScroller.js
98.48% <100%> (ø)
:arrow_up:

source/Masonry/Masonry.js
95.4% <100%> (-0.21%)
:arrow_down:

source/ArrowKeyStepper/ArrowKeyStepper.js
100% <100%> (+2.32%)
:arrow_up:

source/Collection/CollectionView.js
90.65% <86.36%> (+0.65%)
:arrow_up:

source/Grid/Grid.js
94.94% <96%> (+0.18%)
:arrow_up:

source/MultiGrid/MultiGrid.js
96.04% <97.72%> (-0.59%)
:arrow_down:

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 2680550...38f2ce8. Read the comment docs.

I think the only component left to migrate is Grid and it's kind of in a halfway done state.

Codecov Report

Merging #1009 into master will increase coverage by 0.02%.
The diff coverage is 95.48%.

@@            Coverage Diff             @@
##           master    #1009      +/-   ##
==========================================
+ Coverage    90.2%   90.23%   +0.02%
==========================================
  Files          59       59
  Lines        1675     1669       -6
==========================================
- Hits         1511     1506       -5
+ Misses        164      163       -1

Impacted Files
Coverage Δ

source/Masonry/Masonry.js
95.4% <100%> (-0.21%)
:arrow_down:

source/ArrowKeyStepper/ArrowKeyStepper.js
100% <100%> (+2.32%)
:arrow_up:

source/WindowScroller/WindowScroller.js
98.48% <100%> (ø)
:arrow_up:

source/Collection/CollectionView.js
90.65% <86.36%> (+0.65%)
:arrow_up:

source/Grid/Grid.js
94.94% <96%> (+0.18%)
:arrow_up:

source/MultiGrid/MultiGrid.js
96.04% <97.72%> (-0.59%)
:arrow_down:

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 2680550...72d7fd1. Read the comment docs.

Since updates to Grid are a lot more involved than the other components, I've decided to revert the changes in this branch and just focus on landing the others. Grid can be updated (by me, or someone else) in a follow up PR. This way I'm not holding up so many things.

Codecov Report

Merging #1009 into master will decrease coverage by 0.02%.
The diff coverage is 95.18%.

@@            Coverage Diff             @@
##           master    #1009      +/-   ##
==========================================
- Coverage    90.2%   90.18%   -0.03%
==========================================
  Files          59       59
  Lines        1675     1660      -15
==========================================
- Hits         1511     1497      -14
+ Misses        164      163       -1

Impacted Files
Coverage Δ

source/Grid/types.js
0% <ø> (ø)
:arrow_up:

source/WindowScroller/WindowScroller.js
98.48% <100%> (ø)
:arrow_up:

source/ArrowKeyStepper/ArrowKeyStepper.js
100% <100%> (+2.32%)
:arrow_up:

source/Masonry/Masonry.js
95.4% <100%> (-0.21%)
:arrow_down:

source/Collection/CollectionView.js
90.65% <86.36%> (+0.65%)
:arrow_up:

source/MultiGrid/MultiGrid.js
96.04% <97.72%> (-0.59%)
:arrow_down:

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 2680550...72d7fd1. Read the comment docs.

Codecov Report

Merging #1009 into master will decrease coverage by 0.08%.
The diff coverage is 94.52%.

@@            Coverage Diff             @@
##           master    #1009      +/-   ##
==========================================
- Coverage    90.2%   90.12%   -0.09%
==========================================
  Files          59       59
  Lines        1675     1650      -25
==========================================
- Hits         1511     1487      -24
+ Misses        164      163       -1

Impacted Files
Coverage Δ

source/Grid/types.js
0% <ø> (ø)
:arrow_up:

source/Masonry/Masonry.js
95.4% <100%> (-0.21%)
:arrow_down:

source/WindowScroller/WindowScroller.js
98.48% <100%> (ø)
:arrow_up:

source/ArrowKeyStepper/ArrowKeyStepper.js
100% <100%> (+2.32%)
:arrow_up:

source/Collection/CollectionView.js
90.19% <82.35%> (+0.19%)
:arrow_up:

source/MultiGrid/MultiGrid.js
95.93% <97.43%> (-0.7%)
:arrow_down:

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 2680550...f9adfe6. Read the comment docs.

This PR is now ready for review and testing!

Thanks for the review @wuweiweiwu! I added a couple of responses inline. They're a bit terse because I'm on my mobile

I like how much cleaner became code in a few places.

Oh, shoot. I'm sorry @timhwang21 and @TrySound for merging before addressing your comments. They didn't show up for me until after the merge. ☹

Guys, when is this going to be merged into some npm version?

@dobryanskyy Probably in a week.

@dobryanskyy However it's also depends on https://github.com/bvaughn/react-virtualized/pull/1053, so not sure.
/cc @wuweiweiwu

I see, thanks. You're doing a great product


Add ability to proxy to an upstream sub path

This allows to configure an upstream url with a path like http://example.com/foo and requests to /bar will be proxied to http://example.com/foo/bar.

@bashofmann it has been a while since we requested changes to this PR. While the submission is appreciated, we are not able to merge it without these changes. Because it has been so long, we are going to close the issue. Feel free to reopen with the changes included, if your time permits.


Java and Swift SDKs

I divided it into different files(and increased the levels of headers) by analogy with other SDKs and added warnings about alpha versions.
In addition, I corrected the Iterfaces for Interfaces in Java SDK.

@AntonD3 Pls, the remove ForcedExit and FullExit pages, since we don't usually mention them

When I launched the app I couldn't find the tutorial, make sure to update docs/.vuepress/config.js

Ok, I added it to the config.js.


Flink Standalone 运行不了报错

Search before asking

[ ] I had searched in the issues and found no similar question.

[ ] I had googled my question but i didn't get any help.

[ ] I had read the documentation: ChunJun doc but it didn't help me.

Description

Code of Conduct

[X] I agree to follow this project's Code of Conduct

环境变量设置好之后，报这个问题了

能将报错信息截图完整呢？

我这没有复现出该问题，具体执行路径的内容可以看下视频：https://www.bilibili.com/video/BV1TT41137UV?spm_id_from=333.999.0.0&vd_source=f1ba2b9ca22c992869690d82e9d3411d


Chapter 1 revisions

[x] 1.3 Curriculum - last two point could be more parallel in structure to the other points... not start with a verb
[x] 1.4 A description of the open case studies philosophy could be added to the text - content ideas could be pulled from the website or manuscript
[x]  <IP_ADDRESS>-<IP_ADDRESS> Stage 1,2, 3 could incorporate the subsections into the descriptions with headers one level down instead of listing them out at the end
[x] maybe add a Table of Contents subheading before the sentence that says "This anatomy is reflected in..."
[x] line breaks using  could be good between the image and table and the content following the table
[x] 1.6.1 the avocado needs to be resolved - just need to see what computers are required to use R3.5 or higher
[x] 1.6.1 OCSdata should be like OCSdata
[x] 1.6.1 would be good to incorporate what @mbreshock finds about time to knit for each case study
[x]  Perhaps 1.6.2 and 1.6.3 should be subheadings under technical requirements?
[x]  should we make references to chapters be hyperlinks? for example "Chapter 2" in 1.6.3 GitHub
[x] 1.7 should we add an iframe of the survey?
[x] 1.8 Adding some text about why someone might want to contact us  would be good - if you have ideas for case studies, you are interested in working with us, etc. etc.
[x] readtime for case studies (added to chapter 3 teaching a full case study section)
[x] describe operating system for compilation time for each case study and use the word "estimate" and that it may vary with other OSs

I assigned this to all of us so whoever gets to it first works... we can keep track of what needs to be done here with the checkboxes.

I think this is done


UI changes of Home screen.

[XD link]
https://xd.adobe.com/view/f11ac530-31c1-477d-adf4-22524b8b7ed3-a449/screen/4c3e1e2f-8673-468c-ab39-f39929fd6e64/
[Issues to be handled -
1, 2, 3, 4, 5, 6, 7, 9, 10, 11.
8 will handled later once we have the new account management in place].

@cakesoft-swati Could we get a more higher-resolution image attached to this issue? I'm having trouble reading many of the items and seeing what exactly they refer to.

https://github.com/bithyve/hexa/pull/2354 is meant to address most of the points here.
A few notes on some of the points, though:

4 & 5) The spacing is set to 14, which is what I’m seeing in the latest design.

6: The spacing here is always going to be somewhat dynamic, because we always want any potential overflowing columns to be peeking inwards.

-Verified this on Dev app v1.4.1(192)

-Verified this on Dev app v1.4.1(192)


Update version of ADOP SonarQube docker image to 0.2.0

SonarQube image was updated:

https://github.com/Accenture/adop-sonar/releases/tag/0.2.0
https://hub.docker.com/r/accenture/adop-sonar/tags/

@nickdgriffin looks like it was some TravisCI failure, as build is "green" now, could you please accept this PR?

I can indeed :)


Ninja output into dumb terminal is not machine parsable

When ninja writes to a smart terminal it writes the status line for each edge when the command starts and then again when the command finishes immediately followed by the command output.  When writing to a dumb terminal it only writes the status line when the command starts, to avoid duplicate status lines in the output.  When building in parallel the dumb output ends up looking like:
status for cmd 1
status for cmd 2
output for cmd 1
output for cmd 2
This prevents automated tooling from parsing out the output from and individual command, for example to associate error messages or warnings with the file that caused them.
PR #999 changes the status printing for dumb terminals to only print when the edge finishes, instead of when the edge starts.  This is a little less useful for a human reading the output as it runs, as you can't see what command is currently running, but I expect all interactive uses of ninja to use a smart terminal.  For dumb terminal outputs, which I expect is normally redirecting to a file, the output will always be
status for cmd 1
output for cmd 1
status for cmd 2
output for cmd 2
Parsers can use a token in NINJA_STATUS to recognize status lines, and assume everything after a status line is output from that command until the next status line is seen.

It's an interesting idea. Not seeing what's currently running is a bit weird, but in interactive mode that happens frequently too. And it keeps things simple, which is nice.
I posted this to the mailing list for discussion here: https://groups.google.com/forum/#!topic/ninja-build/-wQL6d8d0gg

(We really should make interactive mode update with which command is still
running if we're waiting for something...)
On Sun, Aug 30, 2015 at 11:34 AM, Nico Weber<EMAIL_ADDRESS>wrote:

It's an interesting idea. Not seeing what's currently running is a bit
weird, but in interactive mode that happens frequently too. And it keeps
things simple, which is nice.
I posted this to the mailing list for discussion here:
https://groups.google.com/forum/#!topic/ninja-build/-wQL6d8d0gg
—
Reply to this email directly or view it on GitHub
https://github.com/martine/ninja/issues/1010#issuecomment-136173986.

@martine: I've proposed once the 'still running' patch which prints the
name of (one of the) long-rung running command in the interactive mode:
https://github.com/martine/ninja/pull/629.
Using it myself actually.
SEND
Yesterday
Unread
מיטב דש טרייד
סקירה שבועית מאקרו ושווקים 30-08-2015
This month
Unread
BundlePurchases (25)
Avangate Customer Service, PayPal, כללית, dnt-reply, HotNet, קבוצת
ישראכרט, לאומי
קארד ...
Ariel Zeitlin via LinkedIn
(2)
Catch-up
–
LinkedIn Ariel Zeitlin Ariel Zeitlin Co-Founder & CTO at Guardicore Monday
17th looks fine.
June
lock-free
[lock-free] Abridged summary of<EMAIL_ADDRESS>- 2 updates in 1
topic
–
<EMAIL_ADDRESS>Google Groups Today's topic summary View all
topics Pros and Concerns
lock-free
[lock-free] Abridged summary of<EMAIL_ADDRESS>- 5 updates in 2
topics
–
<EMAIL_ADDRESS>Google Groups Today's topic summary View all
topics Pros and Concerns
lock-free
[lock-free] Abridged summary of<EMAIL_ADDRESS>- 1 update in 1
topic
–
<EMAIL_ADDRESS>Google Groups Today's topic summary View all
topics Pros and Concerns
Maxim Kalaev
Office address
–
The office address is: 550 King ST Littleton, MA 01460 Maxim Kalaev
IBM-XIV, Israel
Maxim Kalaev
Route - boston to littleton
–
https://www.google.co.il/maps/dir/Boston+Logan+International+Airport,+Harborside+Drive,+Boston,+MA,+
מיטב דש טרייד
סקירה שבועית מאקרו ושווקים 7-6-2015
ninja-build
Abridged summary of<EMAIL_ADDRESS>- 1 update in 1 topic
–
<EMAIL_ADDRESS>Google Groups Today's topic summary View all
topics Semantics of
vlada
חוברת2.xlsx
May
מיטב דש טרייד
סקירה שבועית מאקרו ושווקים 10-5-2015
Unread
American Express
הודעת החיוב החודשית בדואר אלקטרוני מילת זיהוי:HENHEN
–
אם הקישור לצפייה בהודעת החיוב אינו מוצג כראוי - אנא לחץ כאן מקסים קלייב
שלום, הודעת החיוב החודשית
April
lock-free
[lock-free] Abridged summary of<EMAIL_ADDRESS>- 1 update in 1
topic
–
<EMAIL_ADDRESS>Google Groups Today's topic summary View all
topics MPSC + backoff
Snoozed 4 months ago
Unread
לאומי קארד
מקסים קלייב, פירוט החיובים החודשי ממתין לך באתר לאומי קארד. מילת זיהוי :
חנחנ
–
מקסים קלייב החל מהיום, פירוט החיובים שלך בחשבון שמספרו מסתיים ב- 5894
ימתינו לך באזור האישי. לצפייה
March
illia
(2)
illia shared an album with you.
–
You are invited to view illia's photo album: Bansko (2015 03 13-20) Bansko
(2015 03 13-20) Mar 15
Earlier
Maxim Kalaev
emacs & python
–
http://www.emacswiki.org/emacs/PythonProgrammingInEmacs#toc8
http://superuser.com/questions/589059/
me
http://kartina.tv/
–
http://kartina.tv/ -- Maxim
Maxim Kalaev
book
–
"The 7 habits of highly effective people" by Stephen R. Covey.:
http://www.amazon.com/
Maxim Kalaev
Self-Service Linux book
–
http://www.scribd.com/doc/84665121/99/Global-Constructors-and-Destructors
Maxim Kalaev
The Algorithm Design Manual (Mixed media product) By (author) Steve S.
Skiena --
–
Maxim Kalaev IBM-XIV, Israel NAS Development Team
On Mon, Aug 31, 2015 at 3:50 AM Evan Martin<EMAIL_ADDRESS>wrote:

(We really should make interactive mode update with which command is still
running if we're waiting for something...)
On Sun, Aug 30, 2015 at 11:34 AM, Nico Weber<EMAIL_ADDRESS>wrote:

It's an interesting idea. Not seeing what's currently running is a bit
weird, but in interactive mode that happens frequently too. And it keeps
things simple, which is nice.
I posted this to the mailing list for discussion here:
https://groups.google.com/forum/#!topic/ninja-build/-wQL6d8d0gg
—
Reply to this email directly or view it on GitHub
https://github.com/martine/ninja/issues/1010#issuecomment-136173986.

—
Reply to this email directly or view it on GitHub
https://github.com/martine/ninja/issues/1010#issuecomment-136223684.

I don't feel strongly, but I do sometimes watch sloooow bot output streaming as "pseudo" interactive, which will be running in dumb mode, so it's sort of nice to see what's running. But it's not a big deal if the other uses are important.

@sgraham I do that all the time too. But you currently don't really know what's running either, right? Currently, all running commands have been printed somewhere above, but you don't know what's still active. (To be clear, I'm slightly nervous about this too, but I think in practice it might be ok. And if it isn't, then we can come up with something else then.)

Since #999 is merged, this is fixed for now. If #999 turns out to be annoying, we can reopen this.


Wrong link of basicModel_neutral_lbs_10_207_0_v1.0.0.pkl

It looks like the mentioned model was actually provided in SMPLify, instead of SMPL.

You can download from this repo


Is ${ user.name.first } a Massachusetts resident?

Add information about what it means to be a resident into the subquestion, rather than hiding behind green words or a hyperlink

Follow up on what the stature requires to consider an individual a resident
If there are complicated situations, consider parsing out those situations we should suggest they speak to a lawyer
Perhaps a Y/N/I don't know to reveal more information


Updated PyTorch Lightning, added optional inference/validation loop, added custom loss functions

Updated PyTorch Lightning to 1.6.5.
added the validation loop where we can (optionally) specify validation data, so we can compute validation loss, NRMSE, and SSIM and plot on Tensorboard. Specify with data_val_file and num_val_data_sets in the config (training config has been renamed data_train_file and num_train_data_sets).
added support for L1, L2, and unweighted SSIM loss functions, specify with loss_function in the config.
can now (optionally) specify datasets for inference, and it will automatically reconstruct the specified datasets after training is finished. Specify with data_inference_file, data_inference_output, and num_inference_data_sets in the config.

Thanks for your contribution. Could you please review my individual comments and let me know your thoughts? In addition, could you please update the PR message to be more instructive?
Thanks!

Should be good to go now.

Will look later, but definition of nrmse is norm(x-xhat)/norm(x), where norm is l2-norm. Just call it something else (since you divide by max - min)
Jon
www.jtsense.com

From: Frederic Wang @.>
Sent: Saturday, September 17, 2022 2:30:36 PM
To: utcsilab/deepinpy @.>
Cc: Jon Tamir @.>; Comment @.>
Subject: Re: [utcsilab/deepinpy] Updated PyTorch Lightning, added optional inference/validation loop, added custom loss functions (PR #29)
@fredwang25 commented on this pull request.

In deepinpy/recons/recon.pyhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Futcsilab%2Fdeepinpy%2Fpull%2F29%23discussion_r973620150&data=05|01||ebe3cae070f44a47963b08da98e319c8|31d7e2a5bdd8414e9e97bea998ebdfe1|0|0|637990398405142492|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|3000|||&sdata=ibJP4zRG4xfEZpxippL3EutsfskZXN3Xm2f440f8AfU%3D&reserved=0:

 resid = pred - gt

 return (torch.real(opt.zdot_single_batch(resid)) / torch.real(opt.zdot_single_batch(gt))).sqrt().mean()

+def calc_nrmse_abs(gt, pred):
I'm pretty sure nrmse by definition is normalized (i.e. normalized root mean squared error), without normalization it would simply be the rmse. Sorry for the poor naming, but I can create a toggle between rmse and nrmse.
—
Reply to this email directly, view it on GitHubhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Futcsilab%2Fdeepinpy%2Fpull%2F29%23discussion_r973620150&data=05|01||ebe3cae070f44a47963b08da98e319c8|31d7e2a5bdd8414e9e97bea998ebdfe1|0|0|637990398405142492|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|3000|||&sdata=ibJP4zRG4xfEZpxippL3EutsfskZXN3Xm2f440f8AfU%3D&reserved=0, or unsubscribehttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FACCQC2ZVARCRSJTWPTJFA53V6YL5ZANCNFSM6AAAAAAQEVTZEU&data=05|01||ebe3cae070f44a47963b08da98e319c8|31d7e2a5bdd8414e9e97bea998ebdfe1|0|0|637990398405142492|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|3000|||&sdata=uIygWN1hE7xju%2BjzZdioywupxsc7ZggQkT6AAnAaEcI%3D&reserved=0.
You are receiving this because you commented.Message ID: @.***>

Looks great! As I still haven't put in automatic testing, I will test this tomorrow with python -m unittest to make sure everything is Kosher. Have you already tried to do that?

Sorry for the late response, just updated the unit tests with the new arguments, and everything passes.


Fix popover menu anchor alignment prop

Details
Fixed Issues
$ https://github.com/Expensify/App/issues/19453, https://github.com/Expensify/App/issues/19791
PROPOSAL: https://github.com/Expensify/App/issues/19453#issuecomment-1560148262
Tests

Go to profile settings page /settings/profile - verify that on clicking the photo icon the popover is not covering the profile photo
Go to profile settings page /workspace/:id/settings - verify that on clicking the photo icon the popover is not covering the workspace photo
Go to a workspace details page /workspace/:id - verify that on clicking the 3-dots the popover is not covering the header

[ ] Verify that no errors appear in the JS console

Offline tests
Same as above
QA Steps
Same as above

[ ] 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).

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

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

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

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
[x] I verified that 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] 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 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
Chrome
https://github.com/Expensify/App/assets/68777211/0ade3100-d0a0-48ab-a1d7-95955d2d39df
Safari
https://github.com/Expensify/App/assets/68777211/53a2bba8-de10-4d7d-99a5-c7bc36722ff7

Mobile Web - Chrome
https://github.com/Expensify/App/assets/68777211/8b484d1c-b1a5-4e04-ae7c-fd46a8c1d591

Mobile Web - Safari
https://github.com/Expensify/App/assets/68777211/03f3ef9f-2943-409e-a1bb-1c5fa11bd722

Desktop
https://github.com/Expensify/App/assets/68777211/659dabc6-6176-4731-92e1-7e23050d76d3

iOS
https://github.com/Expensify/App/assets/68777211/f5b2d87a-8cb0-4f62-9992-bda4db329686

Android
https://github.com/Expensify/App/assets/68777211/200dbb0e-3360-4360-81ac-a205614dc89c

@huzaifa-99 Conflicts

Reviewer Checklist

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

[ ] I verified the steps for local testing are in the Tests section
[ ] I verified the steps for Staging and/or Production testing are in the QA steps section
[ ] 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)
[ ] 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)

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

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

[ ] 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
[ ] I verified proper code patterns were followed (see Reviewing the code)

[ ] 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).
[ ] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[ ] I verified that comments were added to code that is not self explanatory
[ ] 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.
[ ] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
[ ] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[ ] 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.
[ ] 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.
[ ] I verified the JSDocs style guidelines (in STYLE.md) were followed

[ ] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[ ] I verified that this PR follows the guidelines as stated in the Review Guidelines
[ ] 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)
[ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[ ] 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
[ ] If a new component is created I verified that:

[ ] A similar component doesn't exist in the codebase
[ ] All props are defined accurately and each prop has a /** comment above it */
[ ] The file is named correctly
[ ] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[ ] The only data being stored in the state is data necessary for rendering and nothing else
[ ] 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)
[ ] 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)
[ ] All JSX used for rendering exists in the render method
[ ] 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

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

[ ] 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

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

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

[ ] 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.
[ ] 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)
[ ] 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.
[ ] 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.
[ ] 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.
[ ] 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

@huzaifa-99 There are conflicts again

@huzaifa-99 All looks good, there is slight inconsistency between Desktop & Web

See the top positioning of popover between desktop & web
Desktop

Web

Looks good @iwiznia Thoughts?

@iwiznia
This is kind of an edge case, maybe we can define a separate NOOP method to getVerticalPosition as we do for HeaderGap. Instead of writing a height method which is unnecessary
cc: @huzaifa-99

I like that more.

Or maybe getVerticalOffset?

Yeah  getVerticalOffset  looks good to me, @huzaifa-99 please make necessary changes!

: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/iwiznia in version: 1.3.27-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
failure ❌

🕸 web 🕸
success ✅

🚀 Deployed to staging by https://github.com/iwiznia in version: 1.3.27-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

🚀 Deployed to production by https://github.com/AndrewGable in version: 1.3.27-7 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
failure ❌

🕸 web 🕸
success ✅