Skip to content
This repository has been archived by the owner on Dec 27, 2023. It is now read-only.

Updated the cleartext element #337

Closed
wants to merge 9 commits into from

Conversation

opg7371
Copy link

@opg7371 opg7371 commented Feb 19, 2016

Updated the size of cleartext element in the top.css file
Please review .....:)
#317

@smcgregor
Copy link
Member

Hi Piyush, thanks for tackling this!

Can you post a screen capture and if it looks good I'll give it a test on my laptop and phone?

@opg7371
Copy link
Author

opg7371 commented Feb 21, 2016

sorry for the delay..
cleartext-content
Too much big font will too look like a mess.
Thus i kept it normal.
Please tell me if i should increase the size....

However for a large font-size i have created a pull request....
please review it too........#346

@smcgregor
Copy link
Member

Please open a single pull request for each issue and iterate based on comments until we merge it. If you want to show an alternative font, just post a second picture and describe it.

@smcgregor
Copy link
Member

I think the PR with the larger font might be better, but I would need to see it on a smaller screen (a phone). It might be good to conditionally set the font size based on the screen resolution (using @media rules or another method).

@opg7371
Copy link
Author

opg7371 commented Feb 21, 2016

cleartext-min
This is how it looks on phone with a larger font(23px).
Yes i do agree with you regarding the font-size should change based on screen resolution

@smcgregor
Copy link
Member

I think the larger font looks better.

@opg7371
Copy link
Author

opg7371 commented Feb 21, 2016

This one consist of the larger font-size (23px) and looks like this in the browser.
content-large

Please review...:)

@smcgregor
Copy link
Member

Looks good. Can you squash your commits into a single commit and I will merge?

@opg7371
Copy link
Author

opg7371 commented Feb 23, 2016

Sir i guess i did it wrong...sorry
will be soon updating it now.....
Ignore this squash of commits

Updated the font-size of the cleartext element

Updated the cleartext element
Updated the font-size of the cleartext element

Updated the cleartext element

Updated the font-size of the cleartext element
@opg7371
Copy link
Author

opg7371 commented Feb 25, 2016

A more Updated pull request here ....#354

@opg7371 opg7371 closed this Feb 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants