[GitHub] zeppelin issue #2568: ZEPPELIN-2904 Show Remove Paragraph button upfront
Github user malayhm commented on the issue: https://github.com/apache/zeppelin/pull/2568 Thanks, @Leemoonsoo for the information. I am able to run e2e and it is green: https://user-images.githubusercontent.com/1881135/32818801-70ed2d58-c9eb-11e7-9af6-7fd5f165e12a.png;> I tried running `TEST_SELENIUM`, it keeps opening the browser and failing infinitely: `mmajithia-106-mbp:zep-os mmajithia$ TEST_SELENIUM=true mvn -Pweb-ci -pl .,zeppelin-interpreter,zeppelin-zengine,zeppelin-server,zeppelin-display -Dtest=org.apache.zeppelin.integration.ParagraphActionsIT -DfailIfNoTests=false -DskipRat verify [INFO] Scanning for projects... [WARNING] [WARNING] Some problems were encountered while building the effective model for org.apache.zeppelin:zeppelin-spark-dependencies_2.10:jar:0.8.0-SNAPSHOT [WARNING] 'build.plugins.plugin.(groupId:artifactId)' must be unique but found duplicate declaration of plugin com.googlecode.maven-download-plugin:download-maven-plugin @ line 940, column 15 [WARNING] 'build.plugins.plugin.(groupId:artifactId)' must be unique but found duplicate declaration of plugin com.googlecode.maven-download-plugin:download-maven-plugin @ line 997, column 15 [WARNING] [WARNING] Some problems were encountered while building the effective model for org.apache.zeppelin:zeppelin-spark_2.10:jar:0.8.0-SNAPSHOT [WARNING] 'build.plugins.plugin.(groupId:artifactId)' must be unique but found duplicate declaration of plugin org.scala-tools:maven-scala-plugin @ line 506, column 15 [WARNING] 'build.plugins.plugin.(groupId:artifactId)' must be unique but found duplicate declaration of plugin org.apache.maven.plugins:maven-surefire-plugin @ line 514, column 15 [WARNING] 'build.plugins.plugin.(groupId:artifactId)' must be unique but found duplicate declaration of plugin org.apache.maven.plugins:maven-compiler-plugin @ line 525, column 15 [WARNING] 'build.plugins.plugin.(groupId:artifactId)' must be unique but found duplicate declaration of plugin org.scala-tools:maven-scala-plugin @ line 535, column 15 [WARNING] 'build.plugins.plugin.(groupId:artifactId)' must be unique but found duplicate declaration of plugin org.apache.maven.plugins:maven-surefire-plugin @ line 543, column 15 [WARNING] [WARNING] It is highly recommended to fix these problems because they threaten the stability of your build. [WARNING] [WARNING] For this reason, future Maven versions might no longer support building such malformed projects. [WARNING] [INFO] [INFO] Reactor Build Order: [INFO] [INFO] Zeppelin [INFO] Zeppelin: Interpreter [INFO] Zeppelin: Display system apis [INFO] Zeppelin: Zengine [INFO] Zeppelin: Server [INFO] [INFO] [INFO] Building Zeppelin 0.8.0-SNAPSHOT [INFO] [INFO] [INFO] --- maven-checkstyle-plugin:2.13:check (checkstyle-fail-build) @ zeppelin --- [INFO] [INFO] [INFO] --- maven-resources-plugin:2.7:copy-resources (copy-resources) @ zeppelin --- [INFO] Using 'UTF-8' encoding to copy filtered resources. [INFO] skip non existing resourceDirectory /Users/mmajithia/src/zep-os/../_tools/site [INFO] [INFO] --- maven-enforcer-plugin:1.3.1:enforce (enforce) @ zeppelin --- [INFO] [INFO] --- git-commit-id-plugin:2.2.2:revision (default) @ zeppelin --- [INFO] [INFO] --- maven-remote-resources-plugin:1.5:process (default) @ zeppelin --- [INFO] [INFO] --- maven-remote-resources-plugin:1.5:process (process-remote-resources) @ zeppelin --- [INFO] [INFO] --- flatten-maven-plugin:1.0.0:flatten (flatten) @ zeppelin --- [INFO] Generating flattened POM of project org.apache.zeppelin:zeppelin:pom:0.8.0-SNAPSHOT... [INFO] [INFO] --- maven-dependency-plugin:2.8:copy-dependencies (copy-dependencies) @ zeppelin --- [INFO] [INFO] --- maven-site-plugin:3.4:attach-descriptor (attach-descriptor) @ zeppelin --- [INFO] [INFO] [INFO] Building Zeppelin: Interpreter 0.8.0-SNAPSHOT [INFO] [INFO] [INFO] --- maven-checkstyle-plugin:2.13:check (checkstyle-fail-build) @ zeppelin-interpreter --- [INFO] [INFO] [INFO] --- maven-resources-plugin:2.7:copy-resources (copy-resources) @ zeppelin-interpreter --- [INFO] Using 'UTF-8' encoding to copy filtered resources. [INFO] Copying 17 resources [INFO] [INFO] --- git-commit-id-plugin:2.2.2:revision (default) @ zeppelin-interpreter --- [INFO] [INFO] --- maven-remote-resources-plugin:1.5:process (default) @
[GitHub] zeppelin issue #2568: ZEPPELIN-2904 Show Remove Paragraph button upfront
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2568 Right, we don't have good documentation for TEST_SELENIUM, but WEB_E2E you can find documentation [here](https://github.com/apache/zeppelin/tree/master/zeppelin-web). I'm running test in following way WEB_E2E test 1. Build project (e.g. `mvn package -DskipTests`) 2. Start daemon ``` bin/zeppelin-daemon.sh start ``` 3. Run test ``` cd zeppelin-web npm run e2e ``` TEST_SELENIUM test 1. Build package (e.g. `mvn package -DskipTests`) 2. Start daemon ``` bin/zeppelin-daemon.sh start ``` 3. Run test Run integration test under [org.apache.zeppelin.integration](https://github.com/apache/zeppelin/tree/master/zeppelin-server/src/test/java/org/apache/zeppelin/integration) package. ``` TEST_SELENIUM=true mvn -Pweb-ci -pl .,zeppelin-interpreter,zeppelin-zengine,zeppelin-server,zeppelin-display -Dtest=org.apache.zeppelin.integration.InterpreterIT -DfailIfNoTests=false -DskipRat verify ``` you can change `-Dtest=...` whatever the way you want. Single class, single method, etc. Let me know if you need help on this. ---
[GitHub] zeppelin issue #2568: ZEPPELIN-2904 Show Remove Paragraph button upfront
Github user malayhm commented on the issue: https://github.com/apache/zeppelin/pull/2568 @Leemoonsoo: I tried running selenium tests mentioned in the contributions guide, but it didn't help. https://zeppelin.apache.org/contribution/contributions.html Could you please suggest documentation for WEB_E2E and TEST_SELENIUM test profile run? ---
[GitHub] zeppelin issue #2568: ZEPPELIN-2904 Show Remove Paragraph button upfront
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2568 Tried this branch and looks good. But CI is failing in most profiles. Error does not look like related to this change but since this PR made modification on front-end side, I think it's good idea to make sure some related test profile becomes green (e.g. WEB_E2E, TEST_SELENIUM) @malayhm shell we try make test green? ---
[GitHub] zeppelin issue #2568: ZEPPELIN-2904 Show Remove Paragraph button upfront
Github user malayhm commented on the issue: https://github.com/apache/zeppelin/pull/2568 @Leemoonsoo I have created new icon font with remove and more icons. https://user-images.githubusercontent.com/1881135/32284898-b67dd8ce-bf4e-11e7-8811-7b70029c05fb.png;> At present we will have three different icon fonts with different needs. I think we should consolidate all the three implementations to one and move to thin icons for all the icons. I have created a JIRA for this: https://issues.apache.org/jira/browse/ZEPPELIN-3024 ---
[GitHub] zeppelin issue #2568: ZEPPELIN-2904 Show Remove Paragraph button upfront
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2568 > Do you mind if I replace the Simple Line Icon with a separate icon font with all the thin icons and a few more additions Sounds good. Thanks for taking care. ---
[GitHub] zeppelin issue #2568: ZEPPELIN-2904 Show Remove Paragraph button upfront
Github user malayhm commented on the issue: https://github.com/apache/zeppelin/pull/2568 @Leemoonsoo Yes, Simple line icon has 'close' icon, but it will not match with the paragraph controls. I have looked up the usage of Simple line icons and it seems there are only 18 occurrences for all the icons below: - icon-doc - icon-share-alt - icon-plus - icon-arrow-down - icon-arrow-up - icon-settings - icon-control-pause - icon-control-play - icon-question - icon-trash - icon-notebook - icon-doc As we have already added one more icon in this PR and we may add a few in future. Do you mind if I replace the Simple Line Icon with a separate icon font with all the thin icons and a few more additions? ---
[GitHub] zeppelin issue #2568: ZEPPELIN-2904 Show Remove Paragraph button upfront
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2568 Sorry, I missed this comment. Simple line icon has 'close' icon, cross inside of a circle.  I'm not sure this will be okay along with other icons. @malayhm do you think it's better make separate svg with just cross? ---
[GitHub] zeppelin issue #2568: ZEPPELIN-2904 Show Remove Paragraph button upfront
Github user malayhm commented on the issue: https://github.com/apache/zeppelin/pull/2568 @Leemoonsoo Unfortunately Simple Line Icons doesn't have the cross icon, should we use the separate svg for that as well? ---
[GitHub] zeppelin issue #2568: ZEPPELIN-2904 Show Remove Paragraph button upfront
Github user malayhm commented on the issue: https://github.com/apache/zeppelin/pull/2568 @Leemoonsoo : I will fix the spacing issue. Regarding more icon not visible, it seems we need to add it to the build. I will change the delete icon to be cross rather than trash. ---
[GitHub] zeppelin issue #2568: ZEPPELIN-2904 Show Remove Paragraph button upfront
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2568 Thanks @malayhm for great improvement. Tried this branch and have few feedbacks. 1. looks like it's got more space between buttons. Is it intended? can we keep the same space if there's no special reason?  2. I can not see icon image that brings dropdown menu, like below.  3. Is trash icon make sense for delete? Because of Zeppelin does not store Paragraph in the trash to recover, it may introduce wrong expectation on the feature.  I think current 'X' icon is more appropriate in this action.  what do you think? ---
[GitHub] zeppelin issue #2568: ZEPPELIN-2904 Show Remove Paragraph button upfront
Github user malayhm commented on the issue: https://github.com/apache/zeppelin/pull/2568 @1ambda As per this link, we can use it as there is no mention to give credit to the designer: http://blog.iconfinder.com/license-agreements-used-iconfinder/ ---
[GitHub] zeppelin issue #2568: ZEPPELIN-2904 Show Remove Paragraph button upfront
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/2568 @Madhuka Happy to see the icon. I just searched the license but couldn't find the license. It only says "free for commercial usage". Could you check it can be used even in opensource? - https://docs.google.com/spreadsheets/u/1/d/1E8X2_xmJkkoeZwa1HPNG6jT3ytAZlcAgzTDRX0jDF-Q/pubhtml?gid=0=true - http://support.iconfinder.com/quick-guide-to-iconfinder/license-overview ---
[GitHub] zeppelin issue #2568: ZEPPELIN-2904 Show Remove Paragraph button upfront
Github user malayhm commented on the issue: https://github.com/apache/zeppelin/pull/2568 @1ambda Please see the https://user-images.githubusercontent.com/1881135/30287842-2e72d740-9744-11e7-8468-f9a86766487b.png;> new screenshot with the new icon: ---
[GitHub] zeppelin issue #2568: ZEPPELIN-2904 Show Remove Paragraph button upfront
Github user malayhm commented on the issue: https://github.com/apache/zeppelin/pull/2568 @1ambda It seems this icon is available in icon finder with free commercial use: https://www.iconfinder.com/icons/476329/continued_detail_details_ellipses_more_icon#size=16 What do you think? ---
[GitHub] zeppelin issue #2568: ZEPPELIN-2904 Show Remove Paragraph button upfront
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/2568 @malayhm it's nice if we can remove `More` text and there is no license problem ð ---
[GitHub] zeppelin issue #2568: ZEPPELIN-2904 Show Remove Paragraph button upfront
Github user malayhm commented on the issue: https://github.com/apache/zeppelin/pull/2568 @1ambda This is generally used for hamburger menu in mobile mode. What do you think about a custom icon like this, this is used on Facebook?  ---
[GitHub] zeppelin issue #2568: ZEPPELIN-2904 Show Remove Paragraph button upfront
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/2568 @malayhm What about this icon?  - http://fontawesome.io/icon/bars/ ### Screenshot  ---
[GitHub] zeppelin issue #2568: ZEPPELIN-2904 Show Remove Paragraph button upfront
Github user malayhm commented on the issue: https://github.com/apache/zeppelin/pull/2568 @1ambda > Can we use the thinner icon for remove? I have changed the remove paragraph icon to trash.  >Not sure new setting icon is good or not. > you might propose more icons and reviewer can decide based on the proposed icons. I think the menu indicates more items which we didn't want to show up front. Hence added ellipsis icon from the font-awesome library. We can use variations from Simple Line Icons:  Note: We have to update simple line icons to use new icons ---
[GitHub] zeppelin issue #2568: ZEPPELIN-2904 Show Remove Paragraph button upfront
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/2568 I agree to extract remove icon outside of the setting dropdown. - Can we use the thinner icon for remove? - Not sure new setting icon is good or not. * you might propose more icons and reviewer can decide based on the proposed icons. What do u think? ---
