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

fix: links are direct to asset #2593

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

fix: links are direct to asset #2593

wants to merge 12 commits into from

Conversation

JacobCoffee
Copy link
Member

Description

  • Fixes asset links on sponsor user page

Closes

funny

  • code change: 30 minutes
  • tests: 1.5 hours

@JacobCoffee JacobCoffee changed the title fix: links are direct to assett fix: links are direct to asset Sep 19, 2024
@JacobCoffee JacobCoffee enabled auto-merge (squash) September 20, 2024 17:50
{% for asset in provided_assets %}
<li><b>{{ asset.label }}</b>: <a href="{{ asset.user_view_url }}">View asset</a>.</li>
{% endfor %}
{% for asset in provided_assets %}
Copy link
Member

Choose a reason for hiding this comment

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

We're effectively displaying all the same information here now that was previously available in the users:view_provided_assets view.

Should we just replicate the template logic from

{% for asset in provided_assets %}
<p><b>{{ asset.sponsor_benefit }}</b> benefit provides you with {{ asset.label }}:</p>
{% if asset.polymorphic_ctype.name == "Provided Text" %}
<pre>{{ asset.value|urlize }}</pre>
{% elif asset.polymorphic_ctype.name == "Provided File" %}
<a href="{{ asset.value.url }}">View File</a>
{% else %}
{{ asset.value }}
{% endif %}
<small>{{ asset.help_text }}</small>
<br><br>
{% endfor %}
and remove the "Or you can also click here to view all the assets under the same page" part?

Copy link
Member

Choose a reason for hiding this comment

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

Note only reason I bring up replicating the template logic is to restore the help text and context for each.

Compare

Screenshot 2024-09-23 at 8 28 40 AM

on the existing "view all" page with

Screenshot 2024-09-23 at 8 29 00 AM

On the new dashboard section.

Copy link
Member Author

@JacobCoffee JacobCoffee Sep 23, 2024

Choose a reason for hiding this comment

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

It did add a chunk of text if that is a concern. Keeping the core dashboard leaner feels better imo but @loren-c may like the all-in-one approach :D

Copy link
Member Author

Choose a reason for hiding this comment

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

Current:
image

@ewdurbin Suggested:
image

As an aside I think this could use some rearranging or love because its very squished and would look better some other way

Copy link
Member Author

Choose a reason for hiding this comment

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

Something like
image

Seems one of my main issues comes from

{% block content_attributes %}with-right-sidebar{% endblock %}
because we just leave a ton of empty space when there is no sidebar

Copy link
Member Author

Choose a reason for hiding this comment

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

or
image

Copy link
Contributor

Choose a reason for hiding this comment

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

I do like the 'all in one' approach provided we can make it more manageable and not just a giant list, and I do like Jacob's second version.

Looking at the dashboard with all the information on one page, an ability to sort the required assets by due date or by incomplete/complete would be nice, but that seems like a whole other functionality and just a nice to have.

@JacobCoffee
Copy link
Member Author

Changes in e53f422

Although I am not 100%, my checks into the various user views for the removal ofwith-right-sidebar have shown that everything just fills the empty space instead of wasting it. Have yet to find a page that uses it but I will continue to check.

How it looks ish (latest changes I removed excessive padding as you can see in image 1 but further images have it in screenshot.. has since been fixed)

Details image image image image

transition: background-color 0.2s ease;
}

.btn-link {
Copy link
Member Author

Choose a reason for hiding this comment

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

could maybe make this more fancy

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.

Bug: View provided assets button takes you to the full assets list
3 participants