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

Jump to bottom

Improved deep/full copying logic to address errors and provide user feedback #225

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Improved deep/full copying logic to address errors and provide user feedback #225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6398685
Initial hack at deep-copy improvements for large number of records.
hogpilot Sep 26, 2017
295bc9e
Merge branch 'master' into long-copy-progress
hogpilot Nov 1, 2017
2ec919e
Added new portal.util method `copyHostedFeatureService` for `deepCopy…
hogpilot Nov 18, 2017
a5c21fe
Fixed error where item title included the word "Copying" in rare cases.
hogpilot Nov 18, 2017
0352882
Testing revealed a few problems when going from AGOL to Portal: 1) Fi…
hogpilot Nov 22, 2017
7228270
Fixed error where new item ID wasns't showing up on deep copy (after …
hogpilot Nov 22, 2017
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
930 changes: 9 additions & 921 deletions package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

43 changes: 42 additions & 1 deletion src/css/styles.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ body {
background: #fff;
padding-top: 50px;
overflow-y: hidden;
min-width: 750px;
}

.dropArea {
Expand All @@ -25,12 +26,32 @@ body {
border-style: dashed;
}

#mainContainer {
width: 100%;
position: absolute;
bottom: 0;
}

.itemArea {
background: #fff;
overflow-y: auto;
padding-top: 10px;
}

#itemsContainer {
display: flex;
}

#itemsArea {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a compelling reason for hard-coding layout sizes instead of using the responsive elements in Bootstrap?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, the primary reason is because Bootstrap's grid system presents 2 challenges for our UI:

  1. At certain page widths (the most common from what I've seen), Bootstrap's grid system places a margin on the left/right side of the main content area (centered) and leaves a fair amount of space left unavailable. This is particularly relevant with regards to JSON viewing/editing. The new code removes this unused space.
  2. The Bootstrap grid framework defaults to a float implementation when screen size is reduced (i.e. to a typical mobile size) which either causes the right #dropArea to float down (below the #itemArea , and thus become unreachable) or makes either column unusable because the column elements are too small. It also causes the navbar to grow in height and thus cover up the top portion of either content columns.

Basically, there are lots of issues with our UI/UX css currently, and these changes are meant to address some of them before we get to the point where we can look at a full UI refresh down the road.

Copy link
Contributor

@ecaldwell ecaldwell Nov 21, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

The layout changes you're proposing will have implications across the entire app. They should be broken out to a separate branch and PR so we can have a more focused discussion on those specific changes.

Regarding your comments, screenshots would really be helpful. Both 1 and 2 should be addressable by making better use of Bootstrap's media breakpoints. I'm not opposed to migrating away from those in favor of something else as long as it provides flexibility and responsiveness. Hardcoded css rules make both of those harder.

min-width: 300px;
max-width: 550px;
}

#dropArea {
flex: 1 0 0px;
padding: 10px 15px 0 15px;
}

.accordion-inner {
background: #fff;
}
Expand Down Expand Up @@ -94,6 +115,10 @@ body {
margin-right: 10px;
}

.itemHeader .icon {
margin-left: 0;
}

.edit-controls {
position: absolute;
top: -38px;
Expand Down Expand Up @@ -218,7 +243,6 @@ body {

.itemDetailsDiv {
position: relative;
margin-left: 15px;
font-size: 0.8em;
}

Expand Down Expand Up @@ -247,3 +271,20 @@ body {
table.hljs-ln {
width: 100%;
}

.content-item {
display: flex;
flex-direction: column;
}

.content-item.ui-draggable-dragging {
max-width: 450px;
}

.itemHeader {
display: flex;
}

.itemTitle {
flex: 1 0 20px;
}
5 changes: 3 additions & 2 deletions src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ <h4>Copy</h4>
</div>
</div>
</div>
<div class="container col-md-10 center">
<div id="mainContainer" class="container center">
<!-- About modal -->
<div id="aboutModal" class="modal fade" tabindex="-1" role="dialog" aria-labelledby="aboutModalLabel" aria-hidden="true">
<div class="modal-dialog">
Expand Down Expand Up @@ -446,7 +446,7 @@ <h3 id="deepCopyModalLabel">Copy information</h3>
<div id="itemsArea" class="col-xs-5 itemArea">
<!-- content goes here -->
</div>
<div id="dropArea" class="col-xs-7 itemArea">
<div id="dropArea" class="itemArea">
<!-- dropped items will go here -->
</div>
</div>
Expand All @@ -460,6 +460,7 @@ <h3 id="deepCopyModalLabel">Copy information</h3>
{name: "jquery.ui", location: location.pathname.replace(/\/[^/]*$/, '') + "/js/lib", main: "jquery-ui-1.11.4.min"},
{name: "jquery.bootstrap", location: location.pathname.replace(/\/[^/]*$/, '') + "/js/lib/bootstrap/js", main: "bootstrap-3.3.4.min"},
{name: "bootstrap-shim", location: location.pathname.replace(/\/[^/]*$/, '') + "/js/lib", main: "bootstrap-shim"},
{name: "bootstrap-notify", location: location.pathname.replace(/\/[^/]*$/, '') + "/js/lib", main: "bootstrap-notify.min"},
{name: "mustache", location: location.pathname.replace(/\/[^/]*$/, '') + "/js/lib", main: "mustache-0.7.2"},
{name: "nprogress", location: location.pathname.replace(/\/[^/]*$/, '') + "/js/lib", main: "nprogress-0.1.6"},
{name: "clipboard", location: location.pathname.replace(/\/[^/]*$/, '') + "/js/lib", main: "clipboard-1.5.15.min"},
Expand Down
79 changes: 79 additions & 0 deletions src/js/lib/bootstrap-notify.min.js
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Binary file added src/js/lib/images/ui-bg_glass_100_fdf5ce_1x400.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added src/js/lib/images/ui-bg_glass_65_ffffff_1x400.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading