Skip to content

Conversation

@OtisRed
Copy link
Contributor

@OtisRed OtisRed commented Jul 25, 2018

This component joins offer and if succesful redirects user to offer details.

There is no new imports I think.

No idea how to test it but that could be taken care of in other PR - I think.

@codecov-io
Copy link

codecov-io commented Jul 25, 2018

Codecov Report

Merging #1034 into master will decrease coverage by 0.22%.
The diff coverage is 96%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1034      +/-   ##
=========================================
- Coverage   90.92%   90.7%   -0.23%     
=========================================
  Files         114     116       +2     
  Lines        2701    2755      +54     
  Branches       31      31              
=========================================
+ Hits         2456    2499      +43     
- Misses        241     252      +11     
  Partials        4       4
Impacted Files Coverage Δ
...apps/volontulo/tests/views/api/offers/test_join.py 100% <100%> (ø)
backend/apps/volontulo/views/api.py 85.93% <100%> (-0.26%) ⬇️
frontend/src/app/homepage-offer/offers.service.ts 56.52% <50%> (-0.63%) ⬇️
frontend/src/app/app.component.ts 47.05% <0%> (-2.95%) ⬇️
frontend/src/app/metatags.service.ts 47.61% <0%> (-2.39%) ⬇️
backend/apps/volontulo/factories.py 97.95% <0%> (-0.03%) ⬇️
backend/apps/volontulo/serializers.py 92.51% <0%> (ø) ⬆️
backend/apps/volontulo/tests/common.py 100% <0%> (ø) ⬆️
backend/apps/volontulo/tests/test_factories.py 100% <0%> (ø) ⬆️
... and 4 more

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 c6c293b...dcf8f2f. Read the comment docs.

@OtisRed OtisRed changed the title Offer join form #982 [WIP] Offer join form #982 Sep 5, 2018
Copy link
Contributor

@juliaanholcer juliaanholcer left a comment

Choose a reason for hiding this comment

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

Some minor changes in the template are needed to improve the way this form looks.


export class OfferJoinFormComponent implements OnInit {
public joinForm: FormGroup = this.fb.group({
applicant_email: [''],
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO camelCase will be better for TypeScript.

</div>
<div class="form-group row">
<div>
<button type="submit" class="btn-primary">
Copy link
Contributor

Choose a reason for hiding this comment

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

missed btn class

</div>
<div class="form-group row">
<div>
<button type="submit" class="btn-primary">
Copy link
Contributor

Choose a reason for hiding this comment

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

image
This form doesn't look good - smth is wrong with grid and RWD doesn't work as well.

<div class="form-group row">
<label class="col-form-label" for="{{ email }}">Email:</label>
<div class="col-md-5">
<input disabled class="form-control" formControlName="applicant_email" id="{{ email }}"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

You use {{ }} to bind to email variable, that is not defined in the component, so as a result, you get empty for attributes in HTML (same problem for all other labels). Define them just using strings, and if you define for attribute in a label don't forget to define id' in input`, so focus can be set after clicking on label.

@OtisRed
Copy link
Contributor Author

OtisRed commented Sep 12, 2018

Because of git problems I needed to open this change in another PR. Go to #1066

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants