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

highlight current page #25

Closed
wants to merge 3 commits into from
Closed

Conversation

Mankavelda
Copy link

No description provided.

@elhmn
Copy link
Member

elhmn commented May 6, 2022

Nice work again, I left a comment on the other PR. I think once you fixed it on the other PR you could try to apply the same patch to this one 👌

@@ -31,7 +31,18 @@
<li class="page-item"><a href="{{@root.fullUrl}}&page={{.}}" class="page-link">{{ . }}</a></li>
{{/if}}
{{else}}
<li class="page-item"><a href="{{@root.fullUrl}}?page={{.}}" class="page-link">{{ . }}</a></li>
{{#if (portContains @root.fullUrl "3000") }}
Copy link
Member

Choose a reason for hiding this comment

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

This won't work in production, as the fullUrl will be something different and probably won't use a port

Copy link
Member

@elhmn elhmn left a comment

Choose a reason for hiding this comment

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

I tested the implementation and it does not seem to work, if we change the port. Could you test locally and let me know ?

@@ -17,17 +17,32 @@
<li class="disabled page-item"><a class="page-link">...</a></li>
{{/if}}
{{#each (displayPagesNumber interval current pages)}}
{{#if (ifEqual . current 0)}}
{{#if (ifEqual . current 1 0)}}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{{#if (ifEqual . current 1 0)}}
{{#if (ifEqual . current 1)}}

}

export const not = (a: string): boolean=> {
return (!(a));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return (!(a));
return !a;

Copy link
Author

Choose a reason for hiding this comment

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

Thanks @elhmn. i will make the changes right away!!!

}


export const portContains = (a: string, b:string): boolean => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export const portContains = (a: string, b:string): boolean => {
export const portContains = (a: string, b: string): boolean => {

Comment on lines 21 to +34
<li class="active page-item"><a class="page-link">{{ . }}</a></li>
{{else}}
{{#if @root.hasParams}}
{{#if (contains @root.fullUrl "page")}}
<li class="page-item"><a href="{{{constructUrl @root.fullUrl .}}}" class="page-link">{{ . }}</a></li>
{{#if (ifContains @root.fullUrl .)}}
<li class="page-item active"> <a href="{{{constructUrl @root.fullUrl .}}}" class="page-link">{{ . }}</a></li>
{{else}}
<li class="page-item"><a href="{{{constructUrl @root.fullUrl .}}}" class="page-link">{{ . }}</a></li>
{{/if}}
{{else}}
<li class="page-item"><a href="{{@root.fullUrl}}&page={{.}}" class="page-link">{{ . }}</a></li>
{{/if}}
{{else}}
<li class="page-item"><a href="{{@root.fullUrl}}?page={{.}}" class="page-link">{{ . }}</a></li>
{{#if (portContains @root.fullUrl "3000") }}
Copy link
Member

@elhmn elhmn May 12, 2022

Choose a reason for hiding this comment

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

This seems a little complicated and very difficult to follow, is it possible to simplify this entire logic ?

@rakici rakici closed this Aug 22, 2024
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.

3 participants