-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
4 workflow status badge #39
Conversation
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.
Nice styling, looks professional!
Some alt attributes seem to be misplaced though, they are supposed to go on the img tags as a text alternative if there is a problem loading a picture and/or for people using screen readers.
P tags form paragraphs and are primarily for text, so maybe we could use some other tags instead, e.g: div. But then we will have to set margin/padding by ourselves, so that is something to also take into consideration. The divs could then possibly be put inside a section element to group all the badges.
I also noticed that some links are not working yet. We should either add an href attribute on the a tag, or if we do not want to link anywhere we could just remove those a tags altogether.
I have updated the README with new links and refactored the code. Thanks for the input, I've learned a lot and even more 🥳 |
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.
Looks good, the changes suggested by @Ahlberg-iths looks well implemented, and i like the style of the banners.
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.
The changes looks good, I like the style of the banners aswell.
Added status badges
I got some inspiration from this one