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

Modal scroll fix #273

Merged
merged 11 commits into from
Jan 17, 2020
Merged

Modal scroll fix #273

merged 11 commits into from
Jan 17, 2020

Conversation

jayaike
Copy link
Contributor

@jayaike jayaike commented Jan 15, 2020

I made only the modal content scrollable while the buttons and the title remain fixed.

@pushkalkatara @vkartik97 PTAL

@codecov-io
Copy link

codecov-io commented Jan 15, 2020

Codecov Report

Merging #273 into master will not change coverage.
The diff coverage is 29.72%.

@@           Coverage Diff           @@
##           master     #273   +/-   ##
=======================================
  Coverage   50.66%   50.66%           
=======================================
  Files          66       66           
  Lines        3766     3766           
  Branches      442      442           
=======================================
  Hits         1908     1908           
  Misses       1763     1763           
  Partials       95       95
Impacted Files Coverage Δ
...bliclists/challengelist/challengelist.component.ts 47.57% <ø> (ø) ⬆️
...bmissions/challengeviewallsubmissions.component.ts 31.73% <100%> (ø) ⬆️
src/app/app.component.ts 35.21% <25%> (ø) ⬆️
...lengeleaderboard/challengeleaderboard.component.ts 34.07% <33.33%> (ø) ⬆️
src/app/services/endpoints.service.ts 64.89% <50%> (ø) ⬆️
...rc/app/components/challenge/challenge.component.ts 87.85% <7.69%> (ø) ⬆️
... and 1 more
Impacted Files Coverage Δ
...bliclists/challengelist/challengelist.component.ts 47.57% <ø> (ø) ⬆️
...bmissions/challengeviewallsubmissions.component.ts 31.73% <100%> (ø) ⬆️
src/app/app.component.ts 35.21% <25%> (ø) ⬆️
...lengeleaderboard/challengeleaderboard.component.ts 34.07% <33.33%> (ø) ⬆️
src/app/services/endpoints.service.ts 64.89% <50%> (ø) ⬆️
...rc/app/components/challenge/challenge.component.ts 87.85% <7.69%> (ø) ⬆️
... and 1 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 b6f58f3...920d7d6. Read the comment docs.

Copy link
Member

@krtkvrm krtkvrm left a comment

Choose a reason for hiding this comment

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

Hi @nsjcorps
Can you please post the screenshot in which the changes are reflected

@jayaike
Copy link
Contributor Author

jayaike commented Jan 15, 2020

How do I open the modal @vkartik97

@jayaike
Copy link
Contributor Author

jayaike commented Jan 15, 2020

Screenshot (129)

  • Screenshot of fix

@jayaike
Copy link
Contributor Author

jayaike commented Jan 15, 2020

You can judge by the position of the scroll bar

@Sanji515
Copy link
Member

Hey @nsjcorps there's one more modal for editing the phase details, which is not using the utility/modal. Can you please check this?

@jayaike
Copy link
Contributor Author

jayaike commented Jan 15, 2020

@Sanji515 no problem. Can you take a look at the other pr on code splitting

@jayaike
Copy link
Contributor Author

jayaike commented Jan 15, 2020

Screenshot 1_15_2020 8_51_36 PM

Please take a look @vkartik97 @Sanji515

Copy link
Member

@Sanji515 Sanji515 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix @nsjcorps , LGTM! This can be merged 🎉

@jayaike
Copy link
Contributor Author

jayaike commented Jan 15, 2020

Thanks.

@jayaike
Copy link
Contributor Author

jayaike commented Jan 15, 2020

Did you see the other PR? @Sanji515 #274

@Sanji515
Copy link
Member

Did you see the other PR? @Sanji515

You mean this right? Not yet, looking to it now.

@jayaike
Copy link
Contributor Author

jayaike commented Jan 15, 2020

Yes. That is correct. It's really just so you can see the significant difference if we lazy load @Sanji515

</div>
</div>
<div class="buttons">
<ul style="margin-right: 20px;" class="inline-list pointer">
Copy link
Member

Choose a reason for hiding this comment

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

Please move this style to .scss file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lunayach I have updated the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Sanji515 I submitted my task btw :)

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, @nsjcorps.

@lunayach lunayach merged commit 30423af into Cloud-CV:master Jan 17, 2020
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.

5 participants