-
Notifications
You must be signed in to change notification settings - Fork 16
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
Pagination #58
Pagination #58
Conversation
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.
How does searching work with pagination?
20eee16
to
0ea98c5
Compare
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 pagination seems kinda boilerplate, and repeated 4 times. any way to make that a convenient helper or wrapper function?
rows = query.count() | ||
rows = int(rows // 10 + bool(rows % 10)) |
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.
same comment on the naming of rows as your other pagination pr. this feels like it could be named better
return render_template("main.html", db_files=db_files, | ||
get_date_modified=get_date_modified, s3_bucket=s3_bucket, | ||
auth_dict=auth_dict, harolds=harolds, tour_harolds=tour_harolds, | ||
is_rtp=is_rtp, is_eboard=is_eboard, is_tour_page=False) | ||
is_rtp=is_rtp, is_eboard=is_eboard, is_tour_page=False, | ||
current="", page=page, rows=rows, begin=max(1, page-6), | ||
end=min(page+6, rows) + 1) |
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.
nit: one arg per line. this has gotten too long to be readable like this
current="", page=page, rows=rows, begin=max(1, page-6), | ||
end=min(page+6, rows) + 1) |
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.
same comment about begin and end as your other pagination pr. can this be done in the jinja since you're passing page?
rows = query.count() | ||
rows = int(rows // 10 + bool(rows % 10)) | ||
|
||
if page > rows or page < 1: | ||
return "Page Out of Bounds", 404 | ||
|
||
is_rtp = ldap_is_rtp(auth_dict["uid"]) | ||
is_eboard = ldap_is_eboard(auth_dict["uid"]) | ||
|
||
# Retrieve list of files for templating | ||
db_files = File.query.filter_by(author=auth_dict["uid"]).all() | ||
db_files = query.offset((page-1) * 10).limit(10).all() | ||
harolds = get_harold_list(auth_dict["uid"]) | ||
tour_harolds = get_harold_list("root") | ||
return render_template("main.html", db_files=db_files, | ||
get_file_s3=get_file_s3, get_date_modified=get_date_modified, | ||
s3_bucket=s3_bucket, auth_dict=auth_dict, harolds=harolds, | ||
tour_harolds=tour_harolds, is_rtp=is_rtp, is_eboard=is_eboard, is_tour_page=False) | ||
tour_harolds=tour_harolds, is_rtp=is_rtp, is_eboard=is_eboard, is_tour_page=False, | ||
current="/mine", page=page, rows=rows, begin=max(1, page-6), | ||
end=min(page+6, rows) + 1) |
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.
see above re rows
, begin
, and end
rows = query.count() | ||
rows = int(rows // 10 + bool(rows % 10)) | ||
|
||
if page > rows or page < 1: | ||
return "Page Out of Bounds", 404 | ||
|
||
is_rtp = ldap_is_rtp(auth_dict["uid"]) | ||
is_eboard = ldap_is_eboard(auth_dict["uid"]) | ||
|
||
tour_harolds = get_harold_list("root") | ||
db_files = File.query.filter(File.file_hash.in_(harolds)).all() | ||
db_files = query.offset((page-1) * 10).limit(10).all() | ||
return render_template("main.html", db_files=db_files, | ||
get_date_modified=get_date_modified, s3_bucket=s3_bucket, | ||
auth_dict=auth_dict, harolds=harolds, tour_harolds=tour_harolds, | ||
is_rtp=is_rtp, is_eboard=is_eboard, is_tour_page=False) | ||
is_rtp=is_rtp, is_eboard=is_eboard, is_tour_page=False, | ||
current="/selected", page=page, rows=rows, begin=max(1, page-6), | ||
end=min(page+6, rows) + 1) |
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.
rows
, begin
, end
again
rows = query.count() | ||
rows = int(rows // 10 + bool(rows % 10)) | ||
|
||
if page > rows or page < 1: | ||
return "Page Out of Bounds", 404 | ||
|
||
is_rtp = ldap_is_rtp(auth_dict["uid"]) | ||
is_eboard = ldap_is_eboard(auth_dict["uid"]) | ||
|
||
if is_eboard or is_rtp: | ||
harolds = get_harold_list(auth_dict["uid"]) | ||
tour_harolds = get_harold_list("root") | ||
db_files = File.query.filter(File.file_hash.in_(tour_harolds)).all() | ||
db_files = query.offset((page-1) * 10).limit(10).all() | ||
return render_template("main.html", db_files=db_files, | ||
get_date_modified=get_date_modified, s3_bucket=s3_bucket, | ||
auth_dict=auth_dict, harolds=harolds, tour_harolds=tour_harolds, | ||
is_rtp=is_rtp, is_eboard=is_eboard, is_tour_page=True, is_tour_mode=get_tour_lock_status()) | ||
is_rtp=is_rtp, is_eboard=is_eboard, is_tour_page=True, is_tour_mode=get_tour_lock_status(), | ||
current="/tour_page", page=page, rows=rows, begin=max(1, page-6), end=min(page+6, rows) + 1) |
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.
rows
, begin
, end
again
{% if not loop.index0 is divisibleby 2 %} | ||
</div> | ||
{% endif %} | ||
{% if loop.index0 is divisibleby 2 and loop.last %} | ||
</div> | ||
{% endif %} |
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.
{% if not loop.index0 is divisibleby 2 %} | |
</div> | |
{% endif %} | |
{% if loop.index0 is divisibleby 2 and loop.last %} | |
</div> | |
{% endif %} | |
{% if not loop.index0 is divisibleby 2 or loop.last %} | |
</div> | |
{% endif %} |
nit: this could be a bit more compact I think
{% for num in range(begin, end) %} | ||
<li class="page-item"> | ||
<a class="page-link" href="{{ current }}/{{ num }}">{{ num }}</a> | ||
</li> | ||
{% endfor %} |
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.
plan to do anything special for the current page?
<ul class="pagination pagination-lg justify-content-center"> | ||
{% if page == 1 %} | ||
<li class="page-item disabled"> | ||
<a class="page-link" href="{{ current }}{{ page - 1 }}" tabindex="-1">Previous</a> |
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.
would request.url
work instead of current? doc: https://flask.palletsprojects.com/en/2.0.x/templating/
Since whatever happens here is going to carry over to ComputerScienceHouse/Quotefault#68 , let's work it out there and then I'll just re-do it all here after that one is approved |
Add Pagination so only 10 are loaded at a time. This will prevent lag on load.
based on #57