[uiSettings] Page reload toast should remain on screen longer

[lukeelmers]

If a user changes an advanced setting that specifies requiresPageReload: true, a toast notification will pull up telling them to refresh the page:

However, no specific toastLifeTimeMs is provided for the setting, so it falls back to the notifications:lifetime:info (default 5000ms).

We should consider setting the lifetime for this toast to a higher value, since it feels like the type of thing that should be explicitly dismissed as it requires the user to take action.

[elasticmachine]

Pinging @elastic/kibana-core (Team:Core)

[bfrodsham]

Hello! I'd like to take on this problem. Could you assign me to this issue? Also, I'm fairly new to open source contributions, so any advice/pointers would be welcome.

Also, I've found the contribution guide and I'm currently working on getting everything set up. Does my version of Visual Studio have to be the exact type referenced in the guide? I've currently got 2022 installed.

[pgayvallet]

Hello! I'd like to take on this problem. Could you assign me to this issue?

Sure, done.

Also, I'm fairly new to open source contributions, so any advice/pointers would be welcome.

The toast opening is performed here:

kibana/src/plugins/advanced_settings/public/management_app/components/form/form.tsx

Lines 189 to 193 in 3730dd0

	renderPageReloadToast = () => {
	this.props.toasts.add({
	title: i18n.translate('advancedSettings.form.requiresPageReloadToastDescription', {
	defaultMessage: 'One or more settings require you to reload the page to take effect.',
	}),

The change is just about adding the toastLifeTimeMs option to the call, to override the default value.

@lukeelmers what would be a acceptable value here? 15s ? 30s ?

Does my version of Visual Studio have to be the exact type referenced in the guide?

No, any recent version should be fine.

[lukeelmers]

what would be a acceptable value here? 15s ? 30s ?

Hm, ideally it would just stay up until dismissed, but since toasts don't provide that option (to my knowledge), maybe 30s is high enough?

cc @LeeDr who originally reported this issue to me

[bfrodsham]

Not sure if this is the appropriate place to ask for help, but this is where I'm at so far with the issue:

I've found the file needing to be modified and I've added the line of code that (I think) should resolve the issue. Let me know if I've botched it there.

However, I'm in the process of trying to make a pull request for the change. I made the change on my main fork. I think I understand that I actually need to put this change on a branch from my fork, correct? And then I should be able to use my branch to create a pull request? Let me know if I've got any of that wrong.

In the process of trying to do that, I've run into errors with Git. I'm trying to pull from my fork so that I can see locally the branch that I created remotely on GitHub. When I do the pull, this is what it comes back with:

error: Your local changes to the following files would be overwritten by merge:
        .buildkite/ftr_configs.yml
        .buildkite/scripts/steps/fleet/install_all_packages.sh
        .eslintrc.js
        api_docs/actions.mdx
        api_docs/advanced_settings.mdx
        ... [many other api_docs] ...
        api_docs/kbn_core_execution_context_common.mdx
        api_docs/kbn_co
error: The following untracked working tree files would be removed by merge:
        x-pack/plugins/security_solution/public/common/components/exceptions/viewer/exception_item/exception_details.test.tsx
        x-pack/plugins/security_solution/public/common/components/exceptions/viewer/exception_item/exception_details.tsx
        x-pack/plugins/security_solution/public/common/components/exceptions/viewer/exception_item/exception_entries.test.tsx
        x-pack/plugins/security_solution/public/common/components/exceptions/viewer/exception_item/exception_entries.tsx
        x-pack/plugins/security_solution/public/common/components/exceptions/viewer/exception_item/index.stories.tsx
        x-pack/plugins/security_solution/public/common/components/exceptions/viewer/exception_item/index.test.tsx
        x-pack/plugins/security_solution/public/common/components/exceptions/viewer/exception_item/index.tsx
        x-pack/plugins/security_solution/public/common/components/exceptions/viewer/helpers.test.tsx
        x-pack/plugins/security_solution/public/common/components/exceptions/viewer/helpers.tsx
Please move or remove them before you merge.
Aborting
Updating 6b018f797b9..6efef049607

Could someone advise on what I should do next? I appreciate your patience and the opportunity you're giving me to contribute.

[pgayvallet]

However, I'm in the process of trying to make a pull request for the change. I made the change on my main fork. I think I understand that I actually need to put this change on a branch from my fork, correct? And then I should be able to use my branch to create a pull request? Let me know if I've got any of that wrong.

The correct practice is to create a dedicated branch for the PR on your fork yea. But you can also open a PR from your main branch against elastic:main if you performed the change on your main branch.

so that I can see locally the branch that I created remotely on GitHub

Hum, not sure what you did exactly. The correct workflow is usually to create a local branch from an up-to-date main, and then to push it against your remote

FWIW my usual way of create a PR is the following:

git checkout main
git fetch upstream
git pull --rebase upstream main
git checkout -b my-pr-branch
// perform the change on the `my-pr-branch`
git push
// open PR

This assume the elasic repo was added locally as upstream:

git remote add upstream [email protected]:elastic/kibana.git
git fetch upstream

[afharo]

ideally it would just stay up until dismissed,

Can we use toastLifeTimeMs: Infinity to achieve that? Would that cause any memory leak as it may internally await the timeouts forever?

[elasticmachine]

Pinging @elastic/appex-sharedux (Team:SharedUX)

[willie-hung]

Hello @pgayvallet , I'm a first-time contributor to Kibana and I'm wondering if it's necessary to be assigned to this issue before tackling it. Considering that this is a quick fix, I believe it could expedite the process. Thanks!

[pgayvallet]

@Willie-The-Lord assigning is not a requirement, feel free to just open a PR flagged at closing the current issue and it should be fine. But if you prefer, I could also assign the issue to you, just tell me.

[willie-hung]

Thanks @pgayvallet ! I've just opened a PR, ready for review.