-
Notifications
You must be signed in to change notification settings - Fork 2k
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
#354 Information Architecture Review #386
Conversation
@@ -242,6 +242,8 @@ def make_map(): | |||
m.connect('/dataset/{id}.{format}', action='read') | |||
m.connect('dataset_read', '/dataset/{id}', action='read', | |||
ckan_icon='sitemap') | |||
m.connect('dataset_about', '/dataset/about/{id}', action='about', |
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.
'/dataset/{id}/about'
feels more ckan compliant
I've done my initial review of the code I'll try to do some front end testing later today |
The main issue I've seen is the debug info at the top of the page - either it should be in the blue bit as it was or it can run across the black bar but then it wants to be on one line. Overall it feels a bit cramped and for example on localhost:5000/dashboard I don't like how the grey strip (with dataset and tabs in it) is inconsistently sized Anyhow feel free to ignore my thoughts here - also I really hate the dashboard/profile split and still think we should just have a my stuff set of pages. So fix the issues above and I'll merge |
@johnmartin |
|
||
{% set pkg = c.pkg_dict %} | ||
|
||
{% block breadcrumb_content_selected %} class="active"{% endblock %} |
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.
I think this is wrong I see this text in the breadcrumb eg http://localhost:5000/dataset/kjkjhkjhkj
Also I think the whole Orgs bit of the breadcrumb is wrong ie we see it in non-owned datasets and I'm not sure it behaves like a breadcrumb now as I just clicked on a dataset in /dataset and now I get some org crap I know nothing about - what do you think?
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 problem I'm trying to address with this is: there needs to be a way to communicate if a organization owns a dataset and get to other datasets within said organization. When an organization owns a dataset it is within the parents heir-achy and it's a one to one relationship.
I've tried adding context items to the left sidebar, however that isn't used to communicate this context at the moment and ends up confusing the iA. I've tried putting a global header above the dataset that communicates it's parents ownership (see: http://okfn.johnmart.in/prototypes/#?project=harvest_source&view=edit for context) but that doesn't work when a page has breadcrumbs as it breaks the most to least specific downwards structure of the page.
Using breadcrumbs was the simplest solution.
@tobes Yes, I forgot to remove the route as well. It's gone now. For context: I added the about dataset page because I wanted to have a similar page like the about group and about org pages... but then realised it wasn't needed (as the about information is on the main dataset page). I then removed the |
OK, just finished all of the comments and I think that's it. The FF issue is fixed now as well. Also, I kind of agree with you on the helper text in the sidebar when there is nothing above it clashing with |
I want CKAN 2.0 to be clearer. I want it to be more consistent and concise in the way that it communicates context. The changes mentioned below don't take any features away, they just make them a little simpler to understand.
Here's a brief overview of what I'm going to do:
See #354 for context