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

Provision editing permissions, links #2331

Merged
merged 4 commits into from
Jan 23, 2025
Merged

Conversation

goose-life
Copy link
Contributor

@goose-life goose-life commented Jan 23, 2025

image

To do:

  • Link from document

@longhotsummer
Copy link
Contributor

I think it would make more sense to have the links in the same line as the current document links, like the reader mode links. Perhaps before the reader link?

Copy link

github-actions bot commented Jan 23, 2025

Test Results

569 tests  ±0   569 ✅ ±0   4m 29s ⏱️ ±0s
 60 suites ±0     0 💤 ±0 
 60 files   ±0     0 ❌ ±0 

Results for commit 8155d1c. ± Comparison against base commit 7faf6b8.

♻️ This comment has been updated with latest results.

@goose-life
Copy link
Contributor Author

I think it would make more sense to have the links in the same line as the current document links, like the reader mode links. Perhaps before the reader link?

I thought you might say that 😅 it'll involve a change to the Reader at https://github.com/laws-africa/indigo-la-tradition/blob/master/indigo_la_tradition/js/components/DocumentReader/index.ts#L172 but I think you're right :)

@longhotsummer
Copy link
Contributor

I think it would make more sense to have the links in the same line as the current document links, like the reader mode links. Perhaps before the reader link?

I thought you might say that 😅 it'll involve a change to the Reader at https://github.com/laws-africa/indigo-la-tradition/blob/master/indigo_la_tradition/js/components/DocumentReader/index.ts#L172 but I think you're right :)

Ugh. lol.

Why not add a named block to _work_points_in_time.html that is inside the div on line 72, then indigo and other packages can override that in templates (rather than in JS) to add additional links.

@goose-life
Copy link
Contributor Author

Why not add a named block to _work_points_in_time.html that is inside the div on line 72, then indigo and other packages can override that in templates (rather than in JS) to add additional links.

oops too late I already tweaked the query selector, PR incoming 🙈

@goose-life
Copy link
Contributor Author

image

@goose-life
Copy link
Contributor Author

Provision mode:
image

Whole document:
image

@goose-life
Copy link
Contributor Author

Without permission:
image

image image

{% if provision_eid %}
<li class="breadcrumb-item"><a href="{% url 'choose_document_provision' document.pk %}">{{ provision_eid }}</a></li>
{% if provision_eid or perms.indigo_api.publish_document %}
<li class="breadcrumb-item"><i class="fas fa-list published"></i> <a href="{% url 'choose_document_provision' document.pk %}">{% trans "Provision editor" %}</a></li>
Copy link
Contributor

Choose a reason for hiding this comment

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

I personally think that the eid is a useful part of the breadcrumb, because it's only otherwise visible in the url. But up to you.

longhotsummer
longhotsummer previously approved these changes Jan 23, 2025
@goose-life
Copy link
Contributor Author

Why not add a named block to _work_points_in_time.html that is inside the div on line 72, then indigo and other packages can override that in templates (rather than in JS) to add additional links.

I assumed there was a reason we did this differently than all our other overrides and I didn't want to rush into getting rid of the whole ReaderWorkOverviewShim, but I do now think this would be safe to do! I'll update the two PRs quick

@goose-life goose-life merged commit 5a7bf0f into master Jan 23, 2025
5 checks passed
@goose-life goose-life deleted the provision-editing-perms-links branch January 23, 2025 13:19
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.

2 participants