[GitHub] graceguo-supercat commented on a change in pull request #4760: URL shortner for dashboards
graceguo-supercat commented on a change in pull request #4760: URL shortner for
dashboards
URL:
https://github.com/apache/incubator-superset/pull/4760#discussion_r185330381
##
File path:
superset/assets/javascripts/explore/components/ExploreViewContainer.jsx
##
@@ -157,27 +151,6 @@ class ExploreViewContainer extends React.Component {
}
}
- addHistory({ isReplace = false, title }) {
Review comment:
i feel it's safe to keep query state in browser history.
- we still need to keep url scheme backward compatible right? Even we
display url in browser like /r/123, when user request with old scheme, we
should handle it correctly.
- after query state added into browser history, there is no real long url
request happened unless user refreshed browser or someone accessed the shared
url. So i guess the rate of old url scheme is pretty low.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] graceguo-supercat commented on a change in pull request #4760: URL shortner for dashboards
graceguo-supercat commented on a change in pull request #4760: URL shortner for
dashboards
URL:
https://github.com/apache/incubator-superset/pull/4760#discussion_r184816055
##
File path:
superset/assets/javascripts/explore/components/ExploreViewContainer.jsx
##
@@ -157,27 +151,6 @@ class ExploreViewContainer extends React.Component {
}
}
- addHistory({ isReplace = false, title }) {
Review comment:
In airbnb many users didn't realize there is a share linked button. they
still get used to share their work by directly copy-n-paste url from browser. i
guess this change will be dangerous to them :)
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] graceguo-supercat commented on a change in pull request #4760: URL shortner for dashboards
graceguo-supercat commented on a change in pull request #4760: URL shortner for
dashboards
URL:
https://github.com/apache/incubator-superset/pull/4760#discussion_r184810144
##
File path:
superset/assets/javascripts/explore/components/ExploreViewContainer.jsx
##
@@ -157,27 +151,6 @@ class ExploreViewContainer extends React.Component {
}
}
- addHistory({ isReplace = false, title }) {
Review comment:
if you removed this function, when user playing with chart controls, and
they refresh their page (for any reason, without saving slice), will they lost
the current query state? Note during exploring, not all the query parameters
are saved into slice yet.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] graceguo-supercat commented on a change in pull request #4760: URL shortner for dashboards
graceguo-supercat commented on a change in pull request #4760: URL shortner for
dashboards
URL:
https://github.com/apache/incubator-superset/pull/4760#discussion_r184810144
##
File path:
superset/assets/javascripts/explore/components/ExploreViewContainer.jsx
##
@@ -157,27 +151,6 @@ class ExploreViewContainer extends React.Component {
}
}
- addHistory({ isReplace = false, title }) {
Review comment:
if you removed this function, when user playing with chart controls, and
they refresh their page (for any reason, without saving slice), will they lost
the current query state? not all the query parameters are saved in slice yet.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
