New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
setting save success, scroll to notification #18638
Conversation
update plugin scroll
update id
…to m-18637-plug-scroll
update a function error
@@ -227,7 +227,7 @@ export default defineComponent({ | |||
context: 'success', | |||
type: 'transient', | |||
}); | |||
NotificationsStore.scrollToNotification('generalSettings'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
w/ the new code the way to do this is:
const notificationInstanceId = NotificationsStore.show({
message: translate('CoreAdminHome_PluginSettingsSaveSuccess'),
id: 'generalSettings',
context: 'success',
type: 'transient',
});
NotificationsStore.scrollToNotification(notificationInstanceId);
The reason for this change is because Notification.vue makes an AJAX request in markNotificationAsRead() if an ID is specified, but that isn't needed for notifications added client side only. So if you want to avoid making that request, you don't have to specify the id:
property.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add documentation for this later unless it's desired to revert this to the old behavior.
cc @sgiehl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@diosmosis that makes more sense. updating it now
revert some change convert to ID
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good if tests pass
# Conflicts: # plugins/CorePluginsAdmin/vue/dist/CorePluginsAdmin.umd.js # plugins/CorePluginsAdmin/vue/dist/CorePluginsAdmin.umd.min.js
@diosmosis sorry to bother got a question. when I run Also here, should that be a catch method instead of finally?
|
I've noticed this, seems to be a tsc issue/bug, but I don't know how to fix it. Currently just rebuilding when it happens. It doesn't happen very often for me (once a day at most).
It should be finally, unless you want to show the loading gif if there's an error. (EDIT: "no error" not an error.) |
@diosmosis I saw someone solved it by add this https://www.npmjs.com/package/eslint-import-resolver-typescript |
Thanks, @peterhashair! Do you have info on how they used it, specifically w/ vue CLI? |
@diosmosis haven't try myself, but they claim work like this nklayman/vue-cli-plugin-electron-builder#1059 |
Description:
Fixes: #18637
after setting save success, should scroll to notification
Review