Skip to content
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

coalaonline.html: Change "Add Bear" button font to Roboto #284

Merged
merged 1 commit into from Aug 28, 2018

Conversation

Akhelesh
Copy link
Contributor

This changes the font of the "Add Bear" button on Coala Online
from Times New Roman to Roboto.

Closes #270

For short term contributors: we understand that getting your commits well
defined like we require is a hard task and takes some learning. If you
look to help without wanting to contribute long term there's no need
for you to learn this. Just drop us a message and we'll take care of brushing
up your stuff for merge!

Checklist

  • I read the commit guidelines and I've followed
    them.
  • I ran coala over my code locally. (All commits have to pass
    individually.
    It is not sufficient to have "fixup commits" on your PR,
    our bot will still report the issues for the previous commit.) You will
    likely receive a lot of bot comments and build failures if coala does not
    pass on every single commit!

After you submit your pull request, DO NOT click the 'Update Branch' button.
When asked for a rebase, consult coala.io/rebase
instead.

Please consider helping us by reviewing other peoples pull requests as well:

The more you review, the more your score will grow at coala.io and we will
review your PRs faster!

@@ -146,7 +146,8 @@ <h4>Optional Settings</h4>
<div class="row blue-grey darken-4 white-text z-depth-1 no-margin">
<span>
<a class="waves-effect grey-text text-lighten-2">
<i ng-click="add_bears(section)" class="fa fa-plus-circle bears-icon-small" aria-hidden="true"> &nbsp;ADD BEAR
<i ng-click="add_bears(section)" class="fa fa-plus-circle bears-icon-small" aria-hidden="true">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line contains following spacing inconsistencies:

  • Trailing whitespaces.

Origin: SpaceConsistencyBear, Section: spacing.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp01opxl72/partials/tabs/coalaonline.html
+++ b/tmp/tmp01opxl72/partials/tabs/coalaonline.html
@@ -146,7 +146,7 @@
         <div class="row blue-grey darken-4 white-text z-depth-1 no-margin">
           <span>
             <a class="waves-effect grey-text text-lighten-2">
-              <i ng-click="add_bears(section)" class="fa fa-plus-circle bears-icon-small" aria-hidden="true"> 
+              <i ng-click="add_bears(section)" class="fa fa-plus-circle bears-icon-small" aria-hidden="true">
                 &nbsp;<span style="font-family:'Roboto';">ADD BEAR</span>
               </i>
             </a>

@@ -146,7 +146,8 @@ <h4>Optional Settings</h4>
<div class="row blue-grey darken-4 white-text z-depth-1 no-margin">
<span>
<a class="waves-effect grey-text text-lighten-2">
<i ng-click="add_bears(section)" class="fa fa-plus-circle bears-icon-small" aria-hidden="true"> &nbsp;ADD BEAR
<i ng-click="add_bears(section)" class="fa fa-plus-circle bears-icon-small" aria-hidden="true">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing Whitespace: Trailing white spaces are unnecessary and can complicate diffs: Remove the ' ' at the end of the line.

Origin: HTMLLintBear, Section: html.

@TravisBuddy
Copy link

Travis tests have failed

Hey @Akhelesh,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

1st Build

docker run -v=$(pwd):/app --workdir=/app coala/base coala --ci
Unable to find image 'coala/base:latest' locally
latest: Pulling from coala/base
















Digest: sha256:125d6cf24929d39db47b3ccd83d6ed1b6a231fc7e8afbf70bc4b5bd87653b0ff
Status: Downloaded newer image for coala/base:latest
Executing section global...
Executing section linecount...
Executing section spacing...
|    | [NORMAL] SpaceConsistencyBear:
|    | Line contains following spacing inconsistencies:
|    | - Trailing whitespaces.
|----|    | /app/partials/tabs/coalaonline.html
|    |++++| /app/partials/tabs/coalaonline.html
| 146| 146|         <div class="row blue-grey darken-4 white-text z-depth-1 no-margin">
| 147| 147|           <span>
| 148| 148|             <a class="waves-effect grey-text text-lighten-2">
| 149|    |-              <i ng-click="add_bears(section)" class="fa fa-plus-circle bears-icon-small" aria-hidden="true"> 
|    | 149|+              <i ng-click="add_bears(section)" class="fa fa-plus-circle bears-icon-small" aria-hidden="true">
| 150| 150|                 &nbsp;<span style="font-family:'Roboto';">ADD BEAR</span>
| 151| 151|               </i>
| 152| 152|             </a>
[INFO][12:31:27] Applied 'ShowPatchAction' on 'partials/tabs/coalaonline.html' from 'SpaceConsistencyBear'.
Executing section html...
[WARNING][12:31:27] HTMLLintBear: This result has no patch attached.

partials/tabs/coalaonline.html
| 149| ··············<i·ng-click="add_bears(section)"·class="fa·fa-plus-circle·bears-icon-small"·aria-hidden="true">·
|    | [MAJOR] HTMLLintBear:
|    | Trailing Whitespace: Trailing white spaces are unnecessary and can complicate diffs: Remove the ' ' at the end of the line.
Executing section yml...
Executing section cli...

@@ -146,7 +146,8 @@ <h4>Optional Settings</h4>
<div class="row blue-grey darken-4 white-text z-depth-1 no-margin">
<span>
<a class="waves-effect grey-text text-lighten-2">
<i ng-click="add_bears(section)" class="fa fa-plus-circle bears-icon-small" aria-hidden="true"> &nbsp;ADD BEAR
<i ng-click="add_bears(section)" class="fa fa-plus-circle bears-icon-small" aria-hidden="true">
&nbsp;<span style="font-family:'Roboto';">ADD BEAR</span>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great. You've found the button and changed it.

However, we avoid inline styles.

where is the times new roman coming from?

note that bears-icon-small and similar are defined in http://github.com/coala/coalaCSS

Probably the fix needs to occur there.

And probably the solution is not adding Roboto to bears-icon-small, but requires finding why Times New Roman is being inherited and globally disabling it at the root of the problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The root of the problem here is the .fa class of the font awesome icon. Since the "ADD BEAR" text has been written within the tag, the style "font-family: FontAwesome" automatically gets applied to the text.
At present only the:
h1,
h2,
h3,
h4,
h5,
h6 {
font-family: Roboto;
}

style is the best option from the http://github.com/coala/coalaCSS file to apply the Roboto font-family to the text. But this causes the styles of the materialize.css file heading tags to be applied automatically too.
screenshot from 2018-08-15 22-53-24

We can define in a new class in the CoalaCSS file to attend to this particular issue. What do you suggest?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, new class in coalaCSS sounds good!

@jayvdb
Copy link
Member

jayvdb commented Aug 16, 2018

What about if we changed the order of the CSS files, like moving FA https://github.com/coala/landing-frontend/blob/master/index.html#L35 before coalaCSS ?

@blazeu
Copy link
Member

blazeu commented Aug 17, 2018

What about moving the text outside of the icon? that's usually the way its done.

@Akhelesh
Copy link
Contributor Author

Akhelesh commented Aug 17, 2018

Adding a new class to CoalaCSS and moving the text outside the <i></i> tag does the job. For the time being I added a new class to local copy of CoalaCSS to get the results here:
screenshot from 2018-08-17 13-59-00
If this approach is okay how can I add the new class to CoalaCSS hosted on Github? Also what should I name it?

@ksdme
Copy link
Member

ksdme commented Aug 17, 2018

I agree with @blazeu, you should try moving the button text out of the i tag. Fontawesome does not expect for the button label to be inside icon's tag.

@ksdme
Copy link
Member

ksdme commented Aug 18, 2018

Here, I moved the label text into parent anchor tag from the tag supposed to be used for icon and the results are satisfactory:

image

<a class="waves-effect grey-text text-lighten-2">
  <i ng-click="add_bears(section)" class="fa fa-plus-circle bears-icon-small" aria-hidden="true"></i> ADD BEAR
</a>

If you want to have bold label then add a new class called bold to coalaCSS and apply it to the parent anchor tag here.

@Akhelesh
Copy link
Contributor Author

@ksdme yes it works. Sorry I couldn't get to this solution quick. What should I do to my PR for coalaCSS now? Should I delete it and create a new one?

@ksdme
Copy link
Member

ksdme commented Aug 18, 2018

You can let it be open for now, when this PR gets merged without any changes being required in coalaCSS we can have that closed. Please keep it open for now.

<i ng-click="add_bears(section)" class="fa fa-plus-circle bears-icon-small" aria-hidden="true"> &nbsp;ADD BEAR
</i>
<i ng-click="add_bears(section)" class="fa fa-plus-circle bears-icon-small" aria-hidden="true"></i>
&nbsp;ADD BEAR
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

&nbsp; not required, the icon has enough padding.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am very sorry I dragged this issue so long because of my small mistakes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't worry, this shows how insistent you are on learning.

@@ -146,8 +146,8 @@ <h4>Optional Settings</h4>
<div class="row blue-grey darken-4 white-text z-depth-1 no-margin">
<span>
<a class="waves-effect grey-text text-lighten-2">
<i ng-click="add_bears(section)" class="fa fa-plus-circle bears-icon-small" aria-hidden="true"> &nbsp;ADD BEAR
</i>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last request for change, doesn't require this change, you can keep this in a separate line.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarification: I mean </i> can be left in a new line.

Copy link
Contributor Author

@Akhelesh Akhelesh Aug 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't catch that, means icon tags do not have to be on the same line. Am I right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah.

Copy link
Member

@ksdme ksdme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

coala is always spelled with a lower case c, please fix your commit message.

This changes the font of the "Add Bear" button on coala online
from Times New Roman to Roboto.

Closes coala#270
@jayvdb
Copy link
Member

jayvdb commented Aug 25, 2018

ack 15d52f8

@jayvdb
Copy link
Member

jayvdb commented Aug 25, 2018

@gitmate-bot ff

@jayvdb
Copy link
Member

jayvdb commented Aug 25, 2018

Hey! I'm GitMate.io! This pull request is being fastforwarded automatically. Please DO NOT push while fastforward is in progress or your changes would be lost permanently ⚠️

@ksdme
Copy link
Member

ksdme commented Aug 25, 2018

@jayvdb Looks like the ff did not go through.

@jayvdb
Copy link
Member

jayvdb commented Aug 28, 2018

@gitmate-bot ff

@jayvdb
Copy link
Member

jayvdb commented Aug 28, 2018

Hey! I'm GitMate.io! This pull request is being fastforwarded automatically. Please DO NOT push while fastforward is in progress or your changes would be lost permanently ⚠️

@jayvdb
Copy link
Member

jayvdb commented Aug 28, 2018

Automated fastforward with GitMate.io was successful! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

"Add Bear" Button : Change font family to Roboto from Times New Roman
6 participants