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

Deeplink to resource - fix #1275 #1289

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -7,6 +7,7 @@
- Ignore celery tasks results except for tasks which require it and lower the default results expiration to 6 hours [#1281](https://github.com/opendatateam/udata/pull/1281)
- Import community resource avatar style from udata-gouvfr [#1288](https://github.com/opendatateam/udata/pull/1288)
- Terms are now handled from markdown and customizable with the `SITE_TERMS_LOCATION` setting. [#1285](https://github.com/opendatateam/udata/pull/1285)
- Deeplink to resource [#1289](https://github.com/opendatateam/udata/pull/1289)

## 1.2.3 (2017-10-27)

Expand Down
34 changes: 28 additions & 6 deletions js/front/dataset/index.js
Expand Up @@ -28,6 +28,8 @@ function parseUrl(url) {
return a;
}

const RESOURCE_REGEX = /^#resource(-community)?-([0-9a-f-]{36})$/;

new Vue({
mixins: [FrontMixin],
components: {
Expand All @@ -43,6 +45,11 @@ new Vue({
this.loadCoverageMap();
this.checkResources();
this.fetchReuses();
if (document.location.hash) {
this.$nextTick(() => { // Wait for data to be binded
this.openResourceFromHash(document.location.hash);
});
}
log.debug('Dataset display page ready');
},
methods: {
Expand All @@ -64,13 +71,16 @@ new Vue({
/**
* Display a resource or a community ressource in a modal
*/
showResource(id, e, isCommunity) {
// Ensure edit button work
if ([e.target, e.target.parentNode].some(el => el.classList.contains('btn-edit'))) return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this has been removed ?

e.preventDefault();
showResource(id, isCommunity) {
const attr = isCommunity ? 'communityResources' : 'resources';
const resource = this.dataset[attr].find(resource => resource['@id'] === id);
this.$modal(ResourceModal, {resource});
const communityPrefix = isCommunity ? '-community' : '';
location.hash = `resource${communityPrefix}-${id}`;
const modal = this.$modal(ResourceModal, {resource});
modal.$on('modal:closed', () => {
// prevent scrolling to top
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this behavior common to all modals on close ?
Then maybe be we should issue a fix in a separate PR for this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, it's for this one only: since I set the location.hash with the current resource hash, I need to unset it when closing the modal. The #_ is just a trick to avoid the top scroll that would be triggered with #.

location.hash = '_';
});
},

/**
Expand Down Expand Up @@ -233,6 +243,18 @@ new Vue({
this._('New tag suggestion to improve metadata'),
this._('Hello,\n\nI propose this new tag: ')
);
}
},

/**
* Open resource modal if corresponding hash in URL.
* /!\ there is a similar function in <discussion-threads> (jumpToHash),
* jump may come from there too.
*/
openResourceFromHash(hash) {
if (RESOURCE_REGEX.test(hash)) {
const [, isCommunity, id] = hash.match(RESOURCE_REGEX);
this.showResource(id, isCommunity);
}
},
}
});
6 changes: 6 additions & 0 deletions less/udata/resource.less
Expand Up @@ -90,6 +90,12 @@
}
}

.list-group-item-link {
float: right;
margin-top: 5px;
margin-right: 5px;
}

.format-label {
width: 52px;
height: 52px;
Expand Down
11 changes: 8 additions & 3 deletions udata/templates/dataset/resource/list-item.html
@@ -1,12 +1,17 @@
{% set resource_format = resource.format|trim|lower or 'data' %}
<div id="resource-{{resource.id}}" class="list-group-item"
<div id="resource{% if resource.from_community %}-community{% endif %}-{{resource.id}}" class="list-group-item"
Copy link
Contributor

Choose a reason for hiding this comment

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

Hum, this distinction comes from this.
As ressources IDs (including community ressources) are unique and behavior is stricly the same, is this really necessary to have distinct ID pattern ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need the distinction to use showResource that expects a isCommunity parameter, apparently in order to fetch the right resource in the JSON-LD.

The only alternative I can think of right now is to try to find the resource by id in both dataset[resources] and dataset[communityResources]. I'm not sure it's a better alternative.

data-checkurl="{{ url_for('api.check_dataset_resource', dataset=dataset.id, rid=resource.id) }}"
@click="showResource('{{resource.id}}', $event, {{ resource.owner is defined|tojson }})">
@click.prevent="showResource('{{resource.id}}', {{ resource.owner is defined|tojson }})">
<div class="format-label pull-left" v-tooltip tooltip-placement="left">
<span class="ellipsis" data-format="{{ resource_format }}">
{{ resource_format }}
</span>
</div>
<span class="list-group-item-link">
<a @click.stop href="#resource{% if resource.from_community %}-community{% endif %}-{{ resource.id }}">
<span class="fa fa-link"></span>
</a>
</span>
<h4 class="list-group-item-heading ellipsis">
<a href="{{resource.url}}">
<span>{{ resource.title or _('Nameless resource') }}</span>
Expand All @@ -28,7 +33,7 @@ <h4 class="list-group-item-heading ellipsis">
{% set edit_path = 'dataset/{id}/resource/{rid}' %}
{% endif %}
<div class="btn-group btn-group-xs tools">
<a class="btn btn-default btn-edit" v-tooltip title="{{ _('Edit') }}"
<a @click.stop class="btn btn-default btn-edit" v-tooltip title="{{ _('Edit') }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

OMG, is this working ? If so I understand the above removal !! 😍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, it should work ;-) At least it does on my machine.

href="{{ url_for('admin.index', path=edit_path.format(id=dataset.id, rid=resource.id)) }}">
<span class="fa fa-pencil"></span>
</a>
Expand Down