Adding sticky headings #53
Conversation
Adding sticky headings
|
Submitted to address #22. |
|
I don't think this works as expected:
Please also modify your JS to match the existing style. |
Made a side menu bar containing topics
|
See now, i just made some changes with shuain's feedback on the issue! |
|
Saw that the indentation changed quite a lot, was this supposed to be the case? You should try to stick to the old code style, unless it's in deep need of refactoring/reformatting. Otherwise, testing on the page uploaded to GCI seems to show that the sidebar does its intended purpose well, but there's a lot of unutilised space on the right side as a result of the adopted one-column layout. Also, just a minor thing - could you reduce the width of the sidebar? It seems kinda large. |
Topics hidden can be seen in bottom now. and further more developments are done.
|
Hidden Topics can be seen in bottom now. and further more developments are done. when they become up they get sticky!! i think its perfectly alright check it. i will send a screen shot too |
|
|
|
This time i didnt change the old style of code too, i inserted my code alone!!. |
|
|
source-browser.html
Outdated
| <script type="text/javascript"> | ||
| setInterval(function(){ | ||
| $( ".stopic" ).each(function(index) { |
There was a problem hiding this comment.
Please adhere to the previous style and remove the spaces within the brackets i.e. change $( ".stopic" ) to $(".stopic").
There was a problem hiding this comment.
sticky Topics
shortly stopics
Changed to old coding style removed spaces
|
My brief testing showed this:
The bottom sticky headings are positioned very low, and I think it'd be better to have them hover such that they touch the bottom of the "frame" (marked out by the scrollbar). This way it becomes more clear that the headings are part of the content. I also suggest adding a translucent black background to the row of the headings so that we are able to see text scrolling behind the headings. Otherwise, pretty good work so far - just take care of your code style! EDIT: Did some further testing, there's a point where the bottom headings start to flash. |
|
|
Fixes according to Jonarthan W's Request.
source-browser.html
Outdated
| @@ -62,6 +87,44 @@ <h1 id="loading" class="m1 center display-none">⌛</h1> | |||
| <a class="fixed right-0 bottom-0 mr2 mb2 text-decoration-none h2" id="refresh" href="#">🔄</a> | |||
|
|
|||
| <script type="text/javascript" src="https://unpkg.com/[email protected]/dist/zepto.js" integrity="sha384-Cp3V2nlfJJ5aA0ctd1PkfNAEMkXM0EGa6RrmEgiv8D6TCaeZJfUFs/PYfR+B5F5H" crossorigin="anonymous"></script> | |||
| <script type="text/javascript"> | |||
| setInterval(function(){ | |||
There was a problem hiding this comment.
Please run this code through Prettier. There are many issues.
source-browser.html
Outdated
| } | ||
| else if ($(this).position().top <= h) { | ||
| $(this).prev().removeClass('stickyb'); | ||
| $(this).prev().css('width', '100%') |
There was a problem hiding this comment.
This method .can be chained with the above.
source-browser.html
Outdated
| setInterval(function(){ | ||
| $('#content').height($(window).height() - $('#navbar').height() - $('.stickTopic').height() - 40); | ||
| $('.repos').each(function(index) { | ||
| var h = $(window).height() - $('.stickTopic').height() - 8; |
There was a problem hiding this comment.
Don't use var Prettier will also fix this.
There was a problem hiding this comment.
Prettier didnt fixed it, but i do, so done! :)
source-browser.html
Outdated
| @@ -72,16 +135,13 @@ <h1 id="loading" class="m1 center display-none">⌛</h1> | |||
| name: topic, | |||
| repoUrl: `https://github.com/${ORGANIZATION}/apertium-${topic}`, | |||
| })); | |||
|
|
|||
There was a problem hiding this comment.
Please undo the deletion of all these new lines.
There was a problem hiding this comment.
This time only one change had made in the old script! so done :)
source-browser.html
Outdated
| TOPICS.forEach(({ topic, name, repoUrl }, i) => { | ||
| const repoList = $('<ul class="repos list-reset mt0">'); | ||
|
|
||
| const repoList = $('<ul id="'+ name +'" class="repos list-reset mt0">'); |
There was a problem hiding this comment.
Please use jQuery's built-in attribute object parameter.
There was a problem hiding this comment.
That change isn't necessary.. So Done ;)
|
See Now, I just fixed everything i think! :) |
Adding sticky headings