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.

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

data library index page #5712

Merged
merged 107 commits into from
Oct 15, 2015
Merged

data library index page #5712

merged 107 commits into from
Oct 15, 2015

Conversation

matallo
Copy link
Contributor

@matallo matallo commented Sep 23, 2015

  • load cards (using Vis collection)
  • implement more
  • implement filters
    • use Leaflet with hovers for the header country navigation
  • add first tests
  • implement preview
  • implement search
  • finish tests

@@ -0,0 +1,10 @@
@import "../old_common/mixins";

Choose a reason for hiding this comment

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

Prefer single quoted strings

@matallo
Copy link
Contributor Author

matallo commented Sep 23, 2015

started the data library index page, inspired by the new explore, I came across the first questions

screen shot 2015-09-23 at 16 18 58

I wrote the filter links as they are in the dashboard, but had to add a new class for the styles, they live outside the list and item in the explore template, though

we should unify this, worth crating an issue?

}
.DataLibrary-text {
color: #333;
font-weight: 200;

Choose a reason for hiding this comment

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

Properties should be ordered color, font-size, font-weight, line-height

Copy link
Contributor Author

Choose a reason for hiding this comment

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

from the shorthand I prefer:

  • font-weight
  • font-size
  • line-height

@CartoDB/frontend

@matallo
Copy link
Contributor Author

matallo commented Sep 24, 2015

summoning @CartoDB/frontend here, per @xavijam request as I don't want to get off track from the decisions originally discussed with @javisantana, also I applied some of the implementations that have already been done for the explore, thinking of the filtering, ordering and load more (to be implemented).

the /data-library resides in datasets view of common-data user, and we show a different layout/view with a feature_flag

https://github.com/CartoDB/cartodb/pull/5712/files#diff-07eeed666445b4ccdc97b529e78e6998R239

thinking about it, and despite this was decided with @javisantana, maybe it's not the best solution (I'm pretty sure it isn't) as this leaves us in a position where data-library is enabled for all users or none, instead of progressively open the feature to users. Thus, we could have a new route as it's been done with explore (correct me if I'm wrong @javierarce) and make proper use of feature flag.

@@ -93,4 +117,4 @@
.u-showOnTablet {
display: inline-block !important;
}
}
}

Choose a reason for hiding this comment

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

Files should end with a trailing newline

@@ -220,6 +229,12 @@ a.Filters-cleanSearch { line-height:32px }
text-decoration: none;
}
}
.Filters-typeLink.Filters-typeLink--grey {
color: #979ea1;

Choose a reason for hiding this comment

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

Color #979ea1 should be written as #979EA1

}
.IconFont--center {
margin-left: -3px;
}

Choose a reason for hiding this comment

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

Files should end with a trailing newline

@matallo matallo changed the title data library index page [DO NOT MERGE] data library index page Oct 15, 2015
@matallo
Copy link
Contributor Author

matallo commented Oct 15, 2015

retest this, please

matallo added a commit that referenced this pull request Oct 15, 2015
@matallo matallo merged commit 924f785 into master Oct 15, 2015
@matallo matallo deleted the 4795-data_library_markup branch October 19, 2015 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants