[ZEPPELIN-2388] Improve the keyboard shortcuts dialog #2274

soralee · 2017-04-21T10:47:59Z

What is this PR for?

This PR is for Improving the keyboard shortcuts dialog form and here is what i improve.

change to table shape
add scrollbar
fixed that text is misaligned when resizing windows

What type of PR is it?

[Bug fix | Improvement]

What is the Jira issue?

ZEPPELIN-2388; Prettify Keyboard Shortcut Dialog

How should this be tested?

Click `Keyboard Shortcuts' icon in a paragraph.
Check Keyboard Shortcuts shape.
Check resizing windows.

Screenshots (if appropriate)

[Before - 1. default]

[Before - 2. text is misaligned when resizing windows]

[Before - 3. not scrollbar]

[After - 1. default]

[After - 2. resizing windows (my image is a little broken.)]

Last improvement dialog shape

Questions:

Does the licenses files need update? No
Is there breaking changes for older versions? No
Does this needs documentation? No

soralee · 2017-04-21T10:51:22Z

@1ambda @Leemoonsoo Could you guys review this PR, please?

1ambda · 2017-04-21T11:00:00Z

really great improvement! let me test and comment soon!

1ambda · 2017-04-21T14:31:59Z

zeppelin-web/src/app/notebook/paragraph/paragraph.css

+  min-height: 16.428571429px;
+  padding: 15px;
+  border-bottom: 1px solid #9cb4c5;
+  background-color: #3071a9;


What about adding border-top-left-radius and *-right-radius here since modal content has border-radius

1ambda · 2017-04-21T14:33:26Z

zeppelin-web/src/app/notebook/paragraph/paragraph.css

+}
+
+.kbd-white {
+  color: #777777;


it would be nice remove color in .kdb-white, since it's hard to recognize.

Before

After (removing color)

1ambda · 2017-04-21T14:36:18Z

zeppelin-web/src/components/modal-shortcut/modal-shortcut.html

@@ -15,292 +15,297 @@
 <div class="modal fade" id="shortcutModal" tabindex="-1" role="dialog" aria-labelledby="myModalLabel" aria-hidden="true">
  <div class="modal-dialog">
    <div class="modal-content">
-      <div class="modal-header">
+      <div class="shortcut-modal-header">
        <button type="button" class="close" data-dismiss="modal"><span aria-hidden="true">&times;</span><span class="sr-only">Close</span></button>


Can we make x button more visible?

1ambda · 2017-04-21T14:38:57Z

zeppelin-web/src/app/notebook/paragraph/paragraph.css

+}
+
+.shortcut-modal-title {
+  color: white;


I think, it's better to add margin-top: 2px, margin-bottom: 2px (or 3px)since it takes too much space. (it's just title).

Before

After

1ambda · 2017-04-21T14:45:48Z

zeppelin-web/src/app/notebook/paragraph/paragraph.css

+  background-color: #d6dde7;
+  border-color: 1px solid #9cb4c5;
+  color: #337ab7;
+  font-size: 14px !important;


Can we adjust font-weight for each divider? For example font-weight: 500 since it's thick as must as the dialog title

Before

After

1ambda · 2017-04-21T14:47:10Z

zeppelin-web/src/app/notebook/paragraph/paragraph.css

+.table-shortcut > tbody > tr > th {
+  background-color: #d6dde7;
+  border-color: 1px solid #9cb4c5;
+  color: #337ab7;


(MINOR) LINE 265: Let's try color: #383a3c (deep gray color)

Before

After

felixcheung · 2017-04-21T19:16:59Z

awesome!

soralee · 2017-04-22T12:13:06Z

Thank you @1ambda @felixcheung for seeing this!
Yeah, I like @1ambda's opinions! Let me update those as @1ambda's comments.

tae-jun · 2017-04-22T12:22:03Z

Cool! Significantly better than before 😄

soralee · 2017-04-23T00:33:28Z

Thanks @tae-jun for seeing 🙂
And, @1ambda I updated this as your comments!

Could you review again, please?

1ambda · 2017-04-23T00:47:18Z

LGTM!!!

felixcheung · 2017-04-23T00:49:34Z

great - just my preference - I actually like the tighter spacing and without line separator look better...

soralee · 2017-04-24T03:11:35Z

@felixcheung Thanks for your opinion 👍
Let me update this like your describe!

soralee · 2017-04-24T04:54:19Z

I updated as @felixcheung suggestion such as the following screenshot!
Could @felixcheung check it again, please?

felixcheung · 2017-04-24T16:04:22Z

Awesome! I like the new spacing and layout

khalidhuseynov · 2017-04-24T16:28:27Z

very nice! LGTM

1ambda · 2017-05-04T05:59:23Z

need to be rebased to make CI green.

soralee · 2017-05-08T02:22:29Z

@1ambda Thanks, I did.

soralee · 2017-05-10T02:47:47Z

CI failed but irrelevant.

Tests in error: 
  SecurityRestApiTest.testGetUserList:69 » JsonSyntax java.lang.IllegalStateExce...
  SecurityRestApiTest.testTicket:55 » JsonSyntax java.lang.IllegalStateException...

1ambda · 2017-05-10T03:08:41Z

LGTM.

soralee · 2017-05-12T10:33:25Z

Ping!

Leemoonsoo · 2017-05-12T12:03:49Z

LGTM and merge to master if no further comments

1ambda reviewed Apr 21, 2017

View reviewed changes

soralee force-pushed the ZEPPELIN-2388_shortcut_key branch from 6ce8954 to 854522c Compare April 23, 2017 00:31

soralee force-pushed the ZEPPELIN-2388_shortcut_key branch from 854522c to a5b038e Compare April 24, 2017 04:47

soralee force-pushed the ZEPPELIN-2388_shortcut_key branch from a5b038e to 7751758 Compare April 29, 2017 12:41

soralee force-pushed the ZEPPELIN-2388_shortcut_key branch from 7751758 to ff0a3ad Compare May 8, 2017 02:22

soralee force-pushed the ZEPPELIN-2388_shortcut_key branch from ff0a3ad to d77f6ac Compare May 8, 2017 08:17

soralee added 3 commits May 12, 2017 13:23

improve the keyboard shortcuts dialog

6d0cee2

modify as comments

fdcf7f2

remove separator and modify tight space

2428787

soralee force-pushed the ZEPPELIN-2388_shortcut_key branch from d77f6ac to 2428787 Compare May 12, 2017 04:23

asfgit closed this in 8056bc9 May 14, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ZEPPELIN-2388] Improve the keyboard shortcuts dialog #2274

[ZEPPELIN-2388] Improve the keyboard shortcuts dialog #2274

soralee commented Apr 21, 2017 •

edited

soralee commented Apr 21, 2017 •

edited

1ambda commented Apr 21, 2017

1ambda Apr 21, 2017 •

edited

1ambda Apr 21, 2017 •

edited

1ambda Apr 21, 2017

1ambda Apr 21, 2017 •

edited

1ambda Apr 21, 2017

1ambda Apr 21, 2017 •

edited

1ambda Apr 21, 2017 •

edited

felixcheung commented Apr 21, 2017

soralee commented Apr 22, 2017

tae-jun commented Apr 22, 2017

soralee commented Apr 23, 2017

1ambda commented Apr 23, 2017

felixcheung commented Apr 23, 2017

soralee commented Apr 24, 2017

soralee commented Apr 24, 2017

felixcheung commented Apr 24, 2017 via email

khalidhuseynov commented Apr 24, 2017

1ambda commented May 4, 2017

soralee commented May 8, 2017

soralee commented May 10, 2017

1ambda commented May 10, 2017

soralee commented May 12, 2017 •

edited

Leemoonsoo commented May 12, 2017

[ZEPPELIN-2388] Improve the keyboard shortcuts dialog #2274

[ZEPPELIN-2388] Improve the keyboard shortcuts dialog #2274

Conversation

soralee commented Apr 21, 2017 • edited

What is this PR for?

What type of PR is it?

What is the Jira issue?

How should this be tested?

Screenshots (if appropriate)

[Before - 1. default]

[Before - 2. text is misaligned when resizing windows]

[Before - 3. not scrollbar]

[After - 1. default]

[After - 2. resizing windows (my image is a little broken.)]

Last improvement dialog shape

Questions:

soralee commented Apr 21, 2017 • edited

1ambda commented Apr 21, 2017

1ambda Apr 21, 2017 • edited

Choose a reason for hiding this comment

1ambda Apr 21, 2017 • edited

Choose a reason for hiding this comment

Before

After (removing color)

1ambda Apr 21, 2017

Choose a reason for hiding this comment

1ambda Apr 21, 2017 • edited

Choose a reason for hiding this comment

1ambda Apr 21, 2017

Choose a reason for hiding this comment

Before

After

1ambda Apr 21, 2017 • edited

Choose a reason for hiding this comment

Before

After

1ambda Apr 21, 2017 • edited

Choose a reason for hiding this comment

Before

After

felixcheung commented Apr 21, 2017

soralee commented Apr 22, 2017

tae-jun commented Apr 22, 2017

soralee commented Apr 23, 2017

1ambda commented Apr 23, 2017

felixcheung commented Apr 23, 2017

soralee commented Apr 24, 2017

soralee commented Apr 24, 2017

felixcheung commented Apr 24, 2017 via email

khalidhuseynov commented Apr 24, 2017

1ambda commented May 4, 2017

soralee commented May 8, 2017

soralee commented May 10, 2017

1ambda commented May 10, 2017

soralee commented May 12, 2017 • edited

Leemoonsoo commented May 12, 2017

soralee commented Apr 21, 2017 •

edited

soralee commented Apr 21, 2017 •

edited

1ambda Apr 21, 2017 •

edited

1ambda Apr 21, 2017 •

edited

After (removing `color`)

1ambda Apr 21, 2017 •

edited

1ambda Apr 21, 2017 •

edited

1ambda Apr 21, 2017 •

edited

soralee commented May 12, 2017 •

edited