Update: program page #851
Update: program page #851
Conversation
fixing issues related to screen resizing
|
@kshitijrajsharma @dakotabenjamin Thanks as always! some fixes to the program page requested by leadership. |
There was a problem hiding this comment.
Some minor typos and a big question about the new javascript code.
Is this code from an existing third party module of some sort? If so we should rather pull it in from a link rather than commit it to code- this will avoid any copyright issues and make it easier to maintain. At present the code in the tailwind-js-carrousel.js file is largely unreadable so I can't review it. It looks like code that has already been minimized.
js/tailwindcss-js-carrousel.js
Outdated
There was a problem hiding this comment.
What is the source of this code? It's difficult to understand. Did you write this or is it from an existing third party source?
There was a problem hiding this comment.
Hi Dakota - I took it from this site https://github.com/tailwindlabs/tailwindcss -- MIT license.
I named the file after the cdn link (https://cdn.tailwindcss.com/) so it is clear where it comes.
I did not load it as CDN directly like the other js files because I needed to make some minor changes to the code.
|
The Typos, I already took note and will correct them on Siteleaf, there has to be a last review by comms on the text on content so its not final. thank you dk :) |
| </div> --> | ||
|
|
||
|
|
||
| <html> |
There was a problem hiding this comment.
This html section looks repetitive, Can you check ?
There was a problem hiding this comment.
Deleted the repeating section
|
You are using tailwind css carrousel. Neat implementation ⭐️ Do mention this in PR and the reason behind for you to import the lib itself . Importing lib is okay , I just want to make sure whoever comes to this PR after will know why you made this decision instead of cdn. One more small comment on css : I noticed your media css on base css, While testing you can have multiple views like phone, tablet and web . Based on which you can make sure carrousel doesn't get carried away when screen size changes . I think a lot of people will use cellphone browsing hot website . + It will be wise to make this carrousel content dynamic so that you will have option to change or implement new carrousel easily later on as request comes in . |
Changes:
Screenshots of the change:
