-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
Show right messages as soon as possible #632
Conversation
x4base
commented
Jun 16, 2016
- Flashed messages should be flushed in every single page, otherwise they will be accumulated and shown in the wrong page, making the users extremely confused.
- The error messages of API calls should be shown immediately, and should not flash any message, because it will always be shown in the wrong page.
@@ -0,0 +1,28 @@ | |||
var $ = window.$ = require('jquery'); |
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 think this belongs in caravel.js or utils.js
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 can move showApiMessage() and toggleCheckbox() into utils.js.
But we still need to include some .entry.js in list.html, right? Should I still include global_functions.entry.js (or maybe rename it to list.entry.js) and call util.toggleChekbox() in this file? Or do you have any other idea?
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.
Or are you only talking about the line you mentioned? I only have to replace
var $ = window.$ = require('jquery');
var jQuery = window.jQuery = $;
with
var $ = require('jquery');
, just don't touch the window variable, right?
I've just found out it shouldn't be in list.html, otherwise the script will be included multiple times if there are multiple inline list views. For example, in the datasource edit page, there are column list tab and metric list tab, so the script will be included twice. I think it should be in base.html, so I named it common.entry.js |