-
Notifications
You must be signed in to change notification settings - Fork 2
Conversation
Threw in a quick go at icons, which works great locally but is completely messed in that html preview, so I'm going to take a look into that. |
Looks to be an issue with the previewer (possibly a caching one - it may be pulling an older version of the css), this works: https://raw.githack.com/StatCan/kubeflow-containers-desktop/landingpage/base/resources/landing_page/index.html Though my CSS was very fast and loose for a quick proof of concept, with text changed to get things to align better (wrt e.g. centering), so there should still be a better way to set things up with a more explicit structure. |
In today's stand-up, @brendangadd said he wanted to go over the HTML for best practices and accessibility before this is pulled in. I'm putting this in Draft until that's done. For anyone reviewing this, note that one way or another the URLs must be dynamically built (but perhaps it is possible to dynamically build a string that's fed into an |
Rebased to sync up with other recent updates. @wg102 is going to take a look. |
While there please see if you can fix the "no newline" git warnings (IIRC Saffa had luck just adding a new line at the end) and I'm going to try to figure out why that keeps happening on my end. |
I modified the code to use 'a' elements instead and verified the navigation using tabs. |
LGTM. @brendangadd? |
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.
In addition to line comments:
- Run JS through eslint
- Run HTML, CSS, JS through Prettier
4b45756
to
a611e34
Compare
First draft of the landing page. The functionality and general concept is there, and if desired I can add on some relatively simple things such as SVG icons, but I am not much of a front end developer so this may warrant being passed up to somebody in UI.
Addresses StatCan/aaw-kubeflow-containers#38