Skip to content
This repository has been archived by the owner on Jul 6, 2020. It is now read-only.

Modified contact us page to the current version of EvalAI #133

Merged
merged 11 commits into from
Jun 14, 2019

Conversation

Sanji515
Copy link
Member

@Sanji515 Sanji515 commented Jun 2, 2019

  • For the right side icon of all input, changed to use font-awesome icon instead of having image for all of them, so for all other input icons like in Login or Signup we'll change that in coming PR's

  • In the html of Input Component, added the input for textarea

  • Used package ngx-textarea-autosize for dynamically changing the height of textarea when input length grows

  • Link to live demo: http://pr-133-evalai.surge.sh

Here's the screenshot:

Screenshot from 2019-06-02 12-29-32

@codecov-io
Copy link

codecov-io commented Jun 2, 2019

Codecov Report

Merging #133 into master will decrease coverage by 0.03%.
The diff coverage is 40%.

@@            Coverage Diff             @@
##           master     #133      +/-   ##
==========================================
- Coverage   53.41%   53.38%   -0.04%     
==========================================
  Files          55       55              
  Lines        2355     2347       -8     
  Branches      252      253       +1     
==========================================
- Hits         1258     1253       -5     
+ Misses       1024     1020       -4     
- Partials       73       74       +1
Impacted Files Coverage Δ
src/app/components/contact/contact.component.ts 78.78% <ø> (+7.67%) ⬆️
...rc/app/components/utility/input/input.component.ts 58.97% <40%> (-0.49%) ⬇️
src/app/services/mock.window.service.ts 83.33% <0%> (-16.67%) ⬇️
Impacted Files Coverage Δ
src/app/components/contact/contact.component.ts 78.78% <ø> (+7.67%) ⬆️
...rc/app/components/utility/input/input.component.ts 58.97% <40%> (-0.49%) ⬇️
src/app/services/mock.window.service.ts 83.33% <0%> (-16.67%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ca8432...6266ec4. Read the comment docs.

@RishabhJain2018
Copy link
Member

@Sanji515 Please resolve the conflicts here.

@Sanji515
Copy link
Member Author

Sanji515 commented Jun 2, 2019

@Sanji515 Please resolve the conflicts here.

@RishabhJain2018 done

@Sanji515
Copy link
Member Author

Sanji515 commented Jun 9, 2019

</div>

<div *ngIf="type=='textarea'">
<textarea autosize class="input-field" (input)="validateInput($event.target.value)" [class.isValid]="isValid" [class.theme-dark]="theme=='dark'" [value]="value" [readonly]="isReadonly"></textarea>
Copy link
Member

Choose a reason for hiding this comment

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

Please try to do this without adding new package. If it is required in multiple places or needed for other features which is not present in Angular then it is fine but for small improvement adding new package is not a good idea.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Shekharrajak I used this package thinking that it would be good if we have 'text-area' in other places in the future.

Copy link
Member

Choose a reason for hiding this comment

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

Cool! Using more packages can slow down the website. Can you check whether it is behaving properly in all the device size or not?

Copy link
Member Author

Choose a reason for hiding this comment

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

@Shekharrajak yeah it's working perfectly, please check if it's working on your side too

<span class="input-bar"></span>
<label [class.is-not-empty]="!isEmpty" [class.theme-dark]="theme=='dark'">{{placeholder}}</label>
<i class="input-icon {{icon}}" *ngIf="isIconPresent"></i>
<div class="input-message" #textValue [class.hidden]="!(((isRequired && isEmpty)||(!isValid && !isEmpty)) && isDirty)">{{message}}</div>
Copy link
Member

Choose a reason for hiding this comment

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

conditions like !(((isRequired && isEmpty)||(!isValid && !isEmpty)) && isDirty) is not looking good and readable. You can create a separate method to check it- which will return True/False.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Sanji515
Copy link
Member Author

@Shekharrajak Please review

Copy link
Member

@Shekharrajak Shekharrajak left a comment

Choose a reason for hiding this comment

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

  • Whenever you add new method or variables, try add some documentation for it for future reference.
  • Always try to add testcases for your changes as many as possible.

For now we can merge it since you need to make changes top of this PR. But I will expect above things in other PRs.

@Shekharrajak Shekharrajak merged commit 851e75f into Cloud-CV:master Jun 14, 2019
@Shekharrajak
Copy link
Member

Also please write better commit messages from next time -- 'suggested changes' doesn't tell anything about commit.

sanketbansal pushed a commit to sanketbansal/EvalAI-ngx that referenced this pull request Jun 29, 2019
* modified contact us page

* minor change

* minor responsive change
sanketbansal pushed a commit to sanketbansal/EvalAI-ngx that referenced this pull request Jul 18, 2019
* modified contact us page

* minor change

* minor responsive change
sanketbansal pushed a commit to sanketbansal/EvalAI-ngx that referenced this pull request Jul 18, 2019
* modified contact us page

* minor change

* minor responsive change
sanketbansal pushed a commit to sanketbansal/EvalAI-ngx that referenced this pull request Jul 19, 2019
* modified contact us page

* minor change

* minor responsive change
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants