feat: correction of navbar links in docs.jenkins.io #164
Conversation
|
The problem here in this repo we cant review the preview of what is happening in jenkins.io or docs.jenkins.io so its test and go way , although i have tried to add test cases important to the pr.. let me know what are your say on this improvement |
src/jio-navbar.ts
Outdated
| this.visibleMenu = -1; | ||
| } | ||
| private isDocsSite = window.location.hostname === 'docs.jenkins.io'; | ||
| private docsVersion = '2.504.x'; |
There was a problem hiding this comment.
This will be a list/array in the near future. Should be make provisions to prepare for that eventuality?
There was a problem hiding this comment.
Thanks for the feedback! Great point for future
How about we update docsVersion to docsVersions and convert it to an array type. I will make sure the implementation defaults to the current version but remains scalable to support multiple versions in the future.
There was a problem hiding this comment.
Maybe we can do this in the property defintion -
@property({type: Array})
docsVersions = ['2.504.x'];
and then modify the getDocsUrl method to :
private getDocsUrl(originalPath: string): string {
const currentVersion = this.docsVersions[0] || '2.504.x';
// ... rest of our logic.....
}
There was a problem hiding this comment.
But it wont have that version selection ability , so maybe I will see now how i can modify the code to have that ability , maybe this PR we should like keep the review on hold for sometime and come back to this when we successfully complete our new side navbar implementation in antora .
What you think about it?
There was a problem hiding this comment.
make provisions to prepare for that eventuality?
Yes the best time for it will be for me to continue finding currently the best way to achieve version selection logic and get it done so you all can review after as i mentioned that new side bar implementation
There was a problem hiding this comment.
Agreed, take sufficient time on thinking about this before deciding on a solution!
There was a problem hiding this comment.
Maybe we could automate the updating of this part too. What would you recommend us do here @gounthar?
There was a problem hiding this comment.
@krisstern Hey , as discussed in our weekly meet, this is one of this week's task . After I complete the pages migration I will finish this part + some small changes in antora for tomorrow radar
There was a problem hiding this comment.
By tomorrow, I believe Bruno reply will also finalise our changes needed + I will inform before I start how I want to update this PR in slack
|
Hi @krisstern I have revamped the pr with changes to handle dynamic version changes |
biru-codeastromer
force-pushed
the
navbar-links
branch
from
915cb44 to
1fe3fe0
Compare
|
Done some more fixes |
|
Finally green 😇 |
|
@biru-codeastromer have you come up with any good idea about automating the versions handling for the navbar yet? |
Hey @krisstern , right now, I’ve moved from hardcoded/static to manually updating the versions logic. Haven’t gotten around the automation logic just yet I was kind of waiting on Bruno’s response since you had tagged him about this earlier. That said, I think it makes sense to add this to this week's goals. We can definitely bring it up in today’s meeting and align on how to move forward from here. |
|
I have updated the approach here as well |
2 participants
Fixes #227
I included a new approach to have the required navbar links correction in docs.jenkins.io ..
Not all pages were wrong ,some of them were incorrect ..corrected them here
This is one of the crucial part of our
Complete Build Retooling of jenkins.ioprojectAlso some broken links I havent corrected since first I have to make those pages as they will be gatsby ..and then finally include them if needed later