Feature/new footer #790
Feature/new footer #790
Conversation
|
I think now this is ready for review. I have done both new footer and font changes in this branch as they were very minor changes. |
|
Rebasing should solve the python tests, fixed on #825 |
sampsapenna
left a comment
sampsapenna
left a comment
There was a problem hiding this comment.
Approving, though I'd recommend a rebase as well before merging 👍🏼
|
I have done the rebase and now python tests are successful. Will you check and merge this to devel @sampsapenna :) |
csc-felipe
left a comment
csc-felipe
left a comment
There was a problem hiding this comment.
This looks great, and we should have the tests passing before merging.
You added new words, and they also need to be added to the dictionary file. You can check the Github Actions details to see which words are missing from there.
Let me know if you need help understanding the spellcheck.
|
ok :) |
hannyle
left a comment
hannyle
left a comment
There was a problem hiding this comment.
The footer and font look nice to me! 👍
I think we can still improve some things:
On my screen, the footer still looks like blocking the content.
And if I resize the window to be smaller, the footer content disappears.
@hannyle Footer content seems like an issue indeed. About the footer blocking content, the content can now be scrolled back into view, at least when I've tried. |
|
I will check the footer content problem as soon as I can. |
|
The issue of footer content disappearing seems to be related to the nav element resizing which I am using for the the footer. I found what is causing and will try to fix it. |
|
I agree that the footer blocking the content needs to be resolved as well, but if it belongs to this PR or not, I don't know. |
|
Both the issues of footer text disappearing and blocking content has been fixed now and also spelling error check also fixed @csc-felipe @hannyle |
| :alt="$t('message.footerMenu.csc')" | ||
| >{{ | ||
| $t("message.footerMenu.csc") | ||
| }}</a> |
There was a problem hiding this comment.
If we could have the codes in their correct lines and indentation, for example this one, it would be nicer to read :)
| <div class="navbar-end" id="rightmenu"> | ||
| <div class="navbar-item smalltext"> | ||
| <a class="linktext" | ||
| href="https://research.csc.fi/-/sd-connect" | ||
| :alt="$t('message.footerMenu.servicedescription')" | ||
| >{{ | ||
| $t("message.footerMenu.servicedescription") | ||
| }}</a> | ||
| </div> |
There was a problem hiding this comment.
Also as an example, this part should have correct indentation, space and lines as well :)
| .navbar{ | ||
| background-color:#DFE1E3; |
There was a problem hiding this comment.
I think since we also have #DFE1E3 in here https://github.com/CSCfi/swift-browser-ui/blob/feature/new-footer/swift_browser_ui_frontend/src/css/csc.scss#L20, maybe we could use it as a variable instead ?
sampsapenna
force-pushed
the
feature/new-footer
branch
from
b6021bd to
44b9ea3
Compare
Description
Added the footer with new design for SD-Connect frontend and also fixed the font of the application. It also fixes the floating footer bug.
Type of change
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Testing