Skip to content
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.

Sign up for GitHub

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

fix SCT-2914 / PUBLIC-1493 - movement of unselected cells #2076

Open
wants to merge 47 commits into
base: develop
Choose a base branch
from
Open

fix SCT-2914 / PUBLIC-1493 - movement of unselected cells #2076

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
1b35148
fix SCT-2914 / PUBLIC-1493 - movement of unselected cells
eapearson Jan 14, 2021
6aad8e9
add more test doc
eapearson Jan 14, 2021
632318d
fix code quality issue
eapearson Jan 14, 2021
a4a9a83
Address github code quality alerts
eapearson Jan 14, 2021
5ae04bc
reformat to make codacy happy
eapearson Jan 14, 2021
f10799f
more codacy / stylelint happiness
eapearson Jan 14, 2021
9b60725
more codacy spam
eapearson Jan 14, 2021
2c7e7fa
improve test case property naming (TEST_CASEX -> TEST_CASE_X); move l…
eapearson Jan 21, 2021
f6e3d09
refactor test data to support case and env defaults
eapearson Jan 21, 2021
b63e27c
don't expose service key
eapearson Jan 21, 2021
9539643
use null, not empty string
eapearson Jan 21, 2021
da58646
move some test utils into new NarrativeTesting class (and file), to c…
eapearson Jan 22, 2021
25ef6fe
refactor integration tests
eapearson Jan 22, 2021
2ce09ea
remove inadvertently re-exported functions
eapearson Jan 22, 2021
09e0a65
remove duplicate code, part 1
eapearson Jan 22, 2021
234228e
move test data to separate json file
eapearson Jan 22, 2021
4170676
change test data to match ci changes
eapearson Jan 23, 2021
22454ca
refactor some common code into functions; fix usage of async expect
eapearson Jan 23, 2021
574ef34
test config should use narrativetest as the test user
eapearson Jan 26, 2021
c26d813
use chrome binary in pupeteer
eapearson Jan 26, 2021
c96a1fc
Merge remote-tracking branch 'origin/develop' into fix-PUBLIC-1493
eapearson May 25, 2021
efbd2bd
add unreleased section to release notes
eapearson May 25, 2021
7ec78de
prettier and linting fixes
eapearson May 26, 2021
b46252a
add eslint pre push check (just files that will be pushed).
eapearson May 26, 2021
44a05cf
add auto-creation of docker network for local image run
eapearson May 26, 2021
6c3dbc1
address sonarcloud issue
eapearson May 26, 2021
74c3772
Merge remote-tracking branch 'origin/develop' into fix-PUBLIC-1493
eapearson May 26, 2021
32cf292
fix regressions due to merge
eapearson May 27, 2021
5fe036f
improve eslint prepush
eapearson May 27, 2021
6d3cac4
trivial changes to trigger need testing commit
eapearson May 27, 2021
5f5e644
eslint warnings return error code
eapearson May 27, 2021
4b31bcc
make eslint happy
eapearson May 27, 2021
dff4d92
make sonarcloud happy
eapearson May 27, 2021
ea4d6d9
more pr code quality fixes
eapearson May 27, 2021
11c8619
more pr code quality fixes
eapearson May 27, 2021
f4d2f55
Merge remote-tracking branch 'origin/develop' into fix-PUBLIC-1493
eapearson Jul 27, 2021
02b67d0
Correct linting errors in files merged from develop (?!). WebStorm no…
eapearson Jul 27, 2021
a7dc8e1
Prettify file.
eapearson Jul 27, 2021
1749f9a
resolve linting errors.
eapearson Jul 27, 2021
3e63351
bump and pin npm versions; works, deterministic (nearly).
eapearson Jul 28, 2021
f678d6d
sort scripts
eapearson Jul 28, 2021
b81e4a4
remove unused parameter
eapearson Jul 28, 2021
a6ddfa6
prettier run pre-push (via husky) should only inspect files being pushed
eapearson Jul 28, 2021
68536e5
use puppeteer's chrome binary; otherwise it is too hard to match chro…
eapearson Jul 28, 2021
d5febdd
Merge remote-tracking branch 'origin/develop' into fix-PUBLIC-1493
eapearson Sep 30, 2021
0d7876c
restore data-testid to userMenu (merge regression)
eapearson Sep 30, 2021
820f334
Merge remote-tracking branch 'origin/develop' into fix-PUBLIC-1493
eapearson Nov 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RELEASE_NOTES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ This is built on the Jupyter Notebook v6.0.2 (more notes will follow).
### Version NEXT
- SCT-2778 - convert data slideout, public tab, refseq data source to use searchapi2/rpc api rather than searchapi2/legacy.
- enhance integration testing support to add service, host, browser, screen size support
- SCT-2914 / PUBLIC-1493 - fix up/down cell movement behavior for unselected cells

### Version 4.3.1
- Fixed problem where code cells could forget their toggled state after saving.
Expand Down
29 changes: 24 additions & 5 deletions kbase-extension/static/kbase/custom/custom.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,16 @@ CSS should go in /kbase-extension/static/kbase/css/kbaseNarrative.css
(or the specific css file if available)
*/
@font-face {
font-family: 'Glyphicons Halflings';
src: url('../../ext_components/bootstrap/dist/fonts/glyphicons-halflings-regular.eot');
src: url('../../ext_components/bootstrap/dist/fonts/glyphicons-halflings-regular.eot?#iefix') format('embedded-opentype'), url('../../ext_components/bootstrap/dist/fonts/glyphicons-halflings-regular.woff2') format('woff2'), url('../../ext_components/bootstrap/dist/fonts/glyphicons-halflings-regular.woff') format('woff'), url('../../ext_components/bootstrap/dist/fonts/glyphicons-halflings-regular.ttf') format('truetype'), url('../../ext_components/bootstrap/dist/fonts/glyphicons-halflings-regular.svg#glyphicons_halflingsregular') format('svg');
font-family: "Glyphicons Halflings";
src: url("../../ext_components/bootstrap/dist/fonts/glyphicons-halflings-regular.eot");
src:
url("../../ext_components/bootstrap/dist/fonts/glyphicons-halflings-regular.eot?#iefix")
format("embedded-opentype"),
url("../../ext_components/bootstrap/dist/fonts/glyphicons-halflings-regular.woff2") format("woff2"),
url("../../ext_components/bootstrap/dist/fonts/glyphicons-halflings-regular.woff") format("woff"),
url("../../ext_components/bootstrap/dist/fonts/glyphicons-halflings-regular.ttf") format("truetype"),
url("../../ext_components/bootstrap/dist/fonts/glyphicons-halflings-regular.svg#glyphicons_halflingsregular")
format("svg");
}

.select2-container--default .select2-results__option--highlighted[aria-selected] {
Expand Down Expand Up @@ -315,12 +322,24 @@ div#notebook {
color: #ccc;
}

.cell.unselected .btn-default:hover {
color: #000;
}

.cell.unselected .kb-cell-toolbar .title-container {
opacity: 0.5;
}

.cell.unselected .kb-cell-toolbar .buttons-container {
opacity: 0.2;
.btn.btn-default.kb-btn-expander.-minimized {
color: rgba(255, 137, 0, 1);
}

.cell.unselected .kb-btn-expander.-minimized {
color: rgba(255, 137, 0, 0.5);
}

.cell.unselected .kb-btn-expander.-minimized:hover {
color: rgba(255, 137, 0, 1);
}

.kb-btn-icon {
Expand Down
6 changes: 3 additions & 3 deletions kbase-extension/static/kbase/js/userMenu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,10 @@ define([
div({style: 'display:inline-block; width: 34px; vertical-align: top;'}, [
span({class: 'fa fa-user', style: 'font-size: 150%; margin-right: 10px;'})
]),
div({style: 'display: inline-block', 'data-element': 'user-label'}, [
displayName,
div({style: 'display: inline-block'}, [
span({'data-element': 'realname'}, displayName),
br(),
i({}, userName)
i({'data-element': 'username'}, userName)
])
])
]),
Expand Down
25 changes: 11 additions & 14 deletions kbase-extension/static/kbase/js/widgets/narrative_core/kbaseCellToolbarMenu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ define([
) {
'use strict';

var t = html.tag,
const t = html.tag,
div = t('div'),
a = t('a'),
button = t('button'),
Expand Down Expand Up @@ -50,11 +50,13 @@ define([
readOnly = Jupyter.narrative.readonly;

function doMoveCellUp() {
Jupyter.notebook.move_cell_up();
const cellIndex = Jupyter.notebook.find_cell_index(cell);
Jupyter.notebook.move_cell_up(cellIndex);
}

function doMoveCellDown() {
Jupyter.notebook.move_cell_down();
const cellIndex = Jupyter.notebook.find_cell_index(cell);
Jupyter.notebook.move_cell_down(cellIndex);
}

function doDeleteCell() {
Expand Down Expand Up @@ -199,10 +201,8 @@ define([
}

function renderOptions(cell, events) {
var toggleMinMax = utils.getCellMeta(cell, 'kbase.cellState.toggleMinMax', 'maximized'),
toggleIcon = (toggleMinMax === 'maximized' ? 'minus' : 'plus'),
dropdownId = html.genId(),
menuItems = [];
const dropdownId = html.genId();
const menuItems = [];

if (cell.cell_type === 'code') {
menuItems.push({
Expand Down Expand Up @@ -403,12 +403,12 @@ define([
])),

(function () {
var toggleMinMax = utils.getCellMeta(cell, 'kbase.cellState.toggleMinMax', 'maximized'),
const toggleMinMax = utils.getCellMeta(cell, 'kbase.cellState.toggleMinMax', 'maximized'),
toggleIcon = (toggleMinMax === 'maximized' ? 'minus' : 'plus'),
color = (toggleMinMax === 'maximized' ? '#000' : 'rgba(255,137,0,1)');
classModifier = (toggleMinMax === 'maximized' ? '-maximized' : '-minimized');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

return button({
type: 'button',
class: 'btn btn-default btn-xs',
class: `btn btn-default btn-xs kb-btn-expander ${classModifier}`,
dataToggle: 'tooltip',
dataPlacement: 'left',
title: true,
Expand All @@ -418,10 +418,7 @@ define([
id: events.addEvent({type: 'click', handler: doToggleMinMaxCell})
}, [
span({
class: 'fa fa-' + toggleIcon + '-square-o fa-lg',
style: {
color: color
}
class: 'fa fa-' + toggleIcon + '-square-o fa-lg'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fa fa-${toggleIcon}-square-o fa-lg?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was already in place, not changed by the PR. The change was to remove the explicit color style.
But maybe I don't know what the question is?
The toggleIcon in this case is the functional part of the font awesome class name, which in this case is either fa-minus-square-o or fa-plus-square-o , so basically toggles between the minus and plus icons.
I was tempted to, but did not, improve that line by making it a template string.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment was just about using a template string. ATM we don't have any policies either way so it's up to you. 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, right, well, I have tried to restrain myself from just changing code to be better. I do try to restrain myself to only changing code if
(a) it is required for the fix
(b) it is a latent bug not described in the bug report
(c) it is seriously required to have to code make sense (a big exception is any jQuery code, which never makes sense.)
(d) it is required for a test or makes a test easier or more reliable, e.g. an attribute to select on
(e) it angers eslint
I'm sure there are others ...

That said, I do sometimes just make code improvements if they bug me enough and I'm in a generous mood.
In this case, string concatenation is old fashioned, but there are literally hundreds of not thousands of locations in the codebase where they are used and can be transformed into template strings :)
It sure is satisfying to do it, though :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(c) - 😁

Template strings can be implemented globally by eslint, should we wish to adopt a policy towards them at some point.

})
]);
}())
Expand Down
Loading