-
Notifications
You must be signed in to change notification settings - Fork 1
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.
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
48 create two column view #60
base: develop
Are you sure you want to change the base?
Conversation
Added css for columns/ rows in the two-column view. The new scss file (map-layout.scss) was created so I could override some css for the map layouts specifically (mostly just the width of the wrapper).
Update GeoJSON link
Update GeoJSON link
@@ -7,5 +7,5 @@ years_published: 1967 | |||
location: Location | |||
nosheets: 6 | |||
infourl: http://resolve.library.ubc.ca/cgi-bin/catsearch?bid=2802600 | |||
geojsonurl: https://github.com/ubc-lib-geo/spatial-indexes/blob/master/canada_britishColumbia_yaletown.geojson | |||
geojsonurl: https://ubc-lib-geo.github.io/spatial-indexes/north-america/canada_britishColumbia_yaletown.geojson |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file URL was changed in the last commit, so it's fixed on develop already. This change can be safely removed.
@@ -7,5 +7,5 @@ years_published: 1980 | |||
location: University of British Columbia, Vancouver, BC | |||
nosheets: 16 | |||
infourl: http://resolve.library.ubc.ca/cgi-bin/catsearch?bid=12149901 | |||
geojsonurl: https://github.com/ubc-lib-geo/spatial-indexes/blob/master/canada_britishColumbia_ubc.geojson | |||
geojsonurl: https://ubc-lib-geo.github.io/spatial-indexes/north-america/canada_britishColumbia_ubc.geojson |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file URL was changed in the last commit, so it's fixed on develop already. This change can be safely removed.
<li>Click on each map listing to see more detail about a particular map.</li> | ||
</ul> | ||
|
||
<div class="row" style="width: 100%; float: left; min-width: 90%;"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's good practice to keep CSS out of any html files and in one place, so that they are easier to find and change later. "id" can be used for one-use styling, and "class" can be reused across multiple elements. I'd move the styling to the map-layout.sccs file that you made.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there Lauren! I have a few suggestions for the layout.
-
Since there are no columns visible on a "default" state of the map tool, I think that the map itself could make better use of the space. For example, what if the map is bigger and centred, and then slides to the side once an area is selected?
-
Similarly, I think that the map should be on the left, and the columns on the right. Since we start from the tool and move to the columns after they are populated, it makes sense to read the page from left to right.
-
I like that you added a new .scss file; our main.scss is getting long, and it's good practice to break the CSS into multiple files based on what parts of the website they style once they get too lengthy. I added a note in the code itself, but I believe that the inline styling for the divs you created should be moved to your new map-layout.scss to make it easier to find and change. Don't forget to give the ids or classes descriptive names so that future editors know what they are looking at!
-
I just realized that the background of the information in the accordion tabs is pure white when expanded. The background of the website is a very light grey, so I think making the background some kind of medium grey might help distinguish the accordion from the background.
-
The tab with "Label/Title/Date Published" should not change color on hover. I think that actually it would work better as a heading, so that it doesn't look clickable at all.
-
I don't have a solution for this one, since it makes sense to have the title be "sticky" on the page, but the scrolling between the small bit of the site heading to get to the "sticky" part of the map info feels kind of jarring.
Don't let this long list discourage you; you have done such good work with the accordion and the styling! I have a feeling @ect123 might make some of these into their own issues as well. I'd focus on fixing the layout and migrating the CSS to the .scss file. It's been lovely working with you, Lauren!
|
||
|
||
|
||
.wrapper { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default header that we're using from Jekyll uses this class already; can you rename this so that it does not override that one?
The two column view- updated map-item layout and the associated css needed. changed the links as well for the two new md files I added because I realized the links were incorrect.