js-and-css-improvement #62

Merged
yarmo merged 11 commits from KiddyTheKid/web:js-and-css-impromement into dev 2 years ago

CSS Improvement:

  • Primary tag modified to look stylish.
  • Added background color for profile header and profile Data.
  • Responsive look improved for larger contents to each corresponding section.

Script.js File:

  • Added function for lettered services capitalization. (ex DNS, XMPP)

Screenshots provided where made using Mozilla Firefox 84.
Also tested on gnome-web-browser

## CSS Improvement: * Primary tag modified to look stylish. * Added background color for profile header and profile Data. * Responsive look improved for larger contents to each corresponding section. ## Script.js File: * Added function for lettered services capitalization. (ex DNS, XMPP) *Screenshots provided where made using Mozilla Firefox 84.* *Also tested on gnome-web-browser*
KiddyTheKid added 3 commits 2 years ago
KiddyTheKid changed title from js-and-css-impromement to js-and-css-improvement 2 years ago
Owner

Hey there! Thanks for the PR, I like what you did with the primary label.

Since there are improvements to different sections (styling with the label; capitalization in JS), it is my opinion that some of these changes would be better in separate PRs. But for this PR, we'll just roll with it!

I have placed a few comments and questions for you.

Hey there! Thanks for the PR, I like what you did with the primary label. Since there are improvements to different sections (styling with the label; capitalization in JS), it is my opinion that some of these changes would be better in separate PRs. But for this PR, we'll just roll with it! I have placed a few comments and questions for you.
yarmo added the
enhancement
label 2 years ago
KiddyTheKid added 1 commit 2 years ago
Poster

This is great, thank you. I'll check them out and for the next time I will separate improvements grouping them by its purpose (style, functionality)

This is great, thank you. I'll check them out and for the next time I will separate improvements grouping them by its purpose (style, functionality)
yarmo added this to the Feature development project 2 years ago
KiddyTheKid added 1 commit 2 years ago
Owner

I'd love to merge and continue this work. I personally see many flaws with the current design.

I still have one open question: keyoxide/web#62/files#issuecomment-167222

Why the 770px? Please post the reply to the comment in the link.

I'd love to merge and continue this work. I personally see many flaws with the current design. I still have one open question: https://codeberg.org/keyoxide/web/pulls/62/files#issuecomment-167222 Why the 770px? Please post the reply to the comment in the link.
KiddyTheKid added 1 commit 2 years ago
KiddyTheKid reviewed 2 years ago
.container {
max-width: 720px;
/*max-width: 720px;*/
max-width: 770px;
Poster

15px Padding was added to #profileData and #profileHeader and this provoked this behavior:

So I changed the width from 720px to 770px but there is a workaround to not change max-width and leave it 720px as done before.
Workaround is applying font-size: .95em to #profileData.
Should I apply this workaround?

Btw, I am not able to see any other comments on the code.

15px Padding was added to #profileData and #profileHeader and this provoked this behavior: ![](https://srv-store5.gofile.io/download/2p61NY/Screenshot_2021-01-10%20Kiddy%20The%20Kid%20-%20Keyoxide.png) So I changed the width from 720px to 770px but there is a workaround to not change max-width and leave it 720px as done before. Workaround is applying font-size: .95em to #profileData. Should I apply this workaround? Btw, I am not able to see any other comments on the code.
yarmo commented 2 years ago
Owner

Sounds good, no need for that workaround but one last question: .container is also used on non-profile page, so these will just get slightly larger?

I had a second comment but I think it somehow disappeared. Maybe the code it referred to was already implemented.

Sounds good, no need for that workaround but one last question: .container is also used on non-profile page, so these will just get slightly larger? I had a second comment but I think it somehow disappeared. Maybe the code it referred to was already implemented.
Poster

Yes, It will be slightly larger.
From this:
https://pasteboard.co/JJcIYlD.png

to this:
https://pasteboard.co/JJcJ8Zz.png

Yes, It will be slightly larger. From this: ![https://pasteboard.co/JJcIYlD.png](https://pasteboard.co/JJcIYlD.png) to this: ![https://pasteboard.co/JJcJ8Zz.png](https://pasteboard.co/JJcJ8Zz.png)
Poster

Sounds good, no need for that workaround but one last question: .container is also used on non-profile page, so these will just get slightly larger?

I had a second comment but I think it somehow disappeared. Maybe the code it referred to was already implemented.

Yeah, couldnt find it anywhere and I wasn't notified at all about the comments... Very weird but ok, I've updated the comment with the links fixed for the screenshots.

> Sounds good, no need for that workaround but one last question: .container is also used on non-profile page, so these will just get slightly larger? > > I had a second comment but I think it somehow disappeared. Maybe the code it referred to was already implemented. Yeah, couldnt find it anywhere and I wasn't notified at all about the comments... Very weird but ok, I've updated the comment with the links fixed for the screenshots.
KiddyTheKid added 1 commit 2 years ago
KiddyTheKid added 1 commit 2 years ago
KiddyTheKid added 1 commit 2 years ago
KiddyTheKid added 1 commit 2 years ago
KiddyTheKid added 1 commit 2 years ago
yarmo merged commit c4a172e527 into dev 2 years ago
Owner

Awesome! Thanks once again for the changes, there will be more visual updates coming soon. PRs are always welcome :)

Awesome! Thanks once again for the changes, there will be more visual updates coming soon. PRs are always welcome :)
KiddyTheKid deleted branch js-and-css-impromement 2 years ago
Poster

Greaat, I'll be lookinf forward to see the new changes.

Greaat, I'll be lookinf forward to see the new changes.
The pull request has been merged as c4a172e527.
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: keyoxide/keyoxide-web#62
Loading…
There is no content yet.