You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by mushketyk <gi...@git.apache.org> on 2016/09/02 22:59:13 UTC
[GitHub] flink pull request #2467: [FLINK-3719][web frontend] Moving the barrier betw...
GitHub user mushketyk opened a pull request:
https://github.com/apache/flink/pull/2467
[FLINK-3719][web frontend] Moving the barrier between graph and stats
Added a moving split barrier between graph and stats.
It looks like this:
Bottom part becomes scrollable if a user makes it too small, the top part is never scrollable.
- [x] General
- The pull request references the related JIRA issue ("[FLINK-XXX] Jira title text")
- The pull request addresses only one issue
- Each commit in the PR has a meaningful commit message (including the JIRA id)
- [x] Documentation
- Documentation has been added for new functionality
- Old documentation affected by the pull request has been updated
- JavaDoc for public methods has been added
- [x] Tests & Build
- Functionality added by the pull request is covered by tests
- `mvn clean verify` has been executed successfully locally or a Travis build has passed
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mushketyk/flink split-panels
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/2467.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2467
----
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2467: [FLINK-3719][web frontend] Moving the barrier between gra...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/2467
This is a useful extension.
I have pinged someone with more web UI experience to take a look at how it is implemented.
Concerning licenses: Where are the png images for the sliders from? Were they self-created?
In the case of other binary files, we usually left a text file in the directory stating the origin and license of the files.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2467: [FLINK-3719][web frontend] Moving the barrier betw...
Posted by mushketyk <gi...@git.apache.org>.
Github user mushketyk commented on a diff in the pull request:
https://github.com/apache/flink/pull/2467#discussion_r87701960
--- Diff: flink-runtime-web/web-dashboard/app/scripts/modules/jobs/jobs.dir.coffee ---
@@ -168,14 +168,15 @@ angular.module('flinkApp')
return
# ----------------------------------------------
-.directive 'split', () ->
- compile: jQuery () ->
- Split(['#canvas', '#job-panel'], (
- sizes: [50, 50]
- direction: 'vertical'
- ))
-
- return
+.directive 'split', () ->
+ return compile: (tElem, tAttrs) ->
+ getId = (elem) -> "#" + elem.id
+ id1 = getId tElem.children().get(0)
+ id2 = getId tElem.children().get(1)
+ Split([id1, id2], (
--- End diff --
Good point. I'll check if this works.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2467: [FLINK-3719][web frontend] Moving the barrier between gra...
Posted by mushketyk <gi...@git.apache.org>.
Github user mushketyk commented on the issue:
https://github.com/apache/flink/pull/2467
Hi @iampeter
I've updated the PR according to your review.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2467: [FLINK-3719][web frontend] Moving the barrier between gra...
Posted by iampeter <gi...@git.apache.org>.
Github user iampeter commented on the issue:
https://github.com/apache/flink/pull/2467
Thanks, that's better @mushketyk, but I think would be even better if you did not identify the elements explicitly by ids ('#canvas', '#job-panel'), but have the directive contain the two splitable parts, i.e.:
```
<split>
<div>Upper part</div>
<div>Lower part</div>
</split>
```
and just use the first and second child to identify them. This way the directive would be reusable and also could be used in other parts of the dashboard.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2467: [FLINK-3719][web frontend] Moving the barrier betw...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/2467
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2467: [FLINK-3719][web frontend] Moving the barrier between gra...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/2467
The license is MIT, so that is fine.
Can you update the LICENSE file with the dependency?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2467: [FLINK-3719][web frontend] Moving the barrier between gra...
Posted by greghogan <gi...@git.apache.org>.
Github user greghogan commented on the issue:
https://github.com/apache/flink/pull/2467
Merging this ...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2467: [FLINK-3719][web frontend] Moving the barrier between gra...
Posted by iampeter <gi...@git.apache.org>.
Github user iampeter commented on the issue:
https://github.com/apache/flink/pull/2467
@mushketyk do make sure that the generated files are up-to-date (if they're not)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2467: [FLINK-3719][web frontend] Moving the barrier between gra...
Posted by mushketyk <gi...@git.apache.org>.
Github user mushketyk commented on the issue:
https://github.com/apache/flink/pull/2467
Hi @iampeter
I've updated the PR according to your comments. Could you please review it again?
Best regards,
Ivan.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2467: [FLINK-3719][web frontend] Moving the barrier between gra...
Posted by mushketyk <gi...@git.apache.org>.
Github user mushketyk commented on the issue:
https://github.com/apache/flink/pull/2467
@iampeter Is there anything else that I should change?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2467: [FLINK-3719][web frontend] Moving the barrier between gra...
Posted by iampeter <gi...@git.apache.org>.
Github user iampeter commented on the issue:
https://github.com/apache/flink/pull/2467
@mushketyk thanks for this feature, very useful indeed.
I'd like to ask you to reconsider some things:
1. First of all, I think it would be good to wrap split.js in an Angular directive
2. I think the `style` attribute could be replaced by CSS
3. I think there is a typo in the port numer (80801)
Let me know your thoughts.
Thanks,
Piotr
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2467: [FLINK-3719][web frontend] Moving the barrier between gra...
Posted by mushketyk <gi...@git.apache.org>.
Github user mushketyk commented on the issue:
https://github.com/apache/flink/pull/2467
Hi @iampeter
Thank you for your review.
I'll update the code according to your suggestions.
Best regards,
Ivan.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #2467: [FLINK-3719][web frontend] Moving the barrier betw...
Posted by iampeter <gi...@git.apache.org>.
Github user iampeter commented on a diff in the pull request:
https://github.com/apache/flink/pull/2467#discussion_r87658530
--- Diff: flink-runtime-web/web-dashboard/app/scripts/modules/jobs/jobs.dir.coffee ---
@@ -168,14 +168,15 @@ angular.module('flinkApp')
return
# ----------------------------------------------
-.directive 'split', () ->
- compile: jQuery () ->
- Split(['#canvas', '#job-panel'], (
- sizes: [50, 50]
- direction: 'vertical'
- ))
-
- return
+.directive 'split', () ->
+ return compile: (tElem, tAttrs) ->
+ getId = (elem) -> "#" + elem.id
+ id1 = getId tElem.children().get(0)
+ id2 = getId tElem.children().get(1)
+ Split([id1, id2], (
--- End diff --
I think that since Split.js accepts HTML elements, we could just write:
```
compile: (tElem, tAttrs) ->
Split(tElem.children(), (
...
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2467: [FLINK-3719][web frontend] Moving the barrier between gra...
Posted by mushketyk <gi...@git.apache.org>.
Github user mushketyk commented on the issue:
https://github.com/apache/flink/pull/2467
Hi @iampeter @StephanEwen
I've fixed the PR according to your review. I've added new license, introduced a new directive, and updated CSS.
Could you please review it once again?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2467: [FLINK-3719][web frontend] Moving the barrier between gra...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/2467
Piotr (who wrote most of the web ui) also wants to leave some comments. Let's wait for him.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2467: [FLINK-3719][web frontend] Moving the barrier between gra...
Posted by mushketyk <gi...@git.apache.org>.
Github user mushketyk commented on the issue:
https://github.com/apache/flink/pull/2467
Hi @iampeter,
Sorry for the delay in response. I'll take a look at this on Sunday.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #2467: [FLINK-3719][web frontend] Moving the barrier between gra...
Posted by mushketyk <gi...@git.apache.org>.
Github user mushketyk commented on the issue:
https://github.com/apache/flink/pull/2467
Hi @StephanEwen
All images are coming from this repo: https://github.com/nathancahill/Split.js
It has a very permissive license: https://github.com/nathancahill/Split.js/blob/master/LICENSE.txt
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---