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.

Sign up for GitHub

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

Jump to bottom

EPMRPP-78105 || Notifications preloader, EPMRPP-78115 || Modal window fix #3162

Merged
merged 2 commits into from
Jul 5, 2022
Merged

EPMRPP-78105 || Notifications preloader, EPMRPP-78115 || Modal window fix #3162

merged 2 commits into from
Jul 5, 2022

Conversation

tr1ble
Copy link
Contributor

@tr1ble tr1ble commented Jul 4, 2022

EPMRPP-78105 || Notifications preloader, EPMRPP-78115 || Modal window paddings fix

@codecov-commenter
Copy link

codecov-commenter commented Jul 4, 2022 •
edited
Loading

Codecov Report

Merging #3162 (37c5905) into develop (ed5df7f) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #3162   +/-   ##
========================================
  Coverage    59.70%   59.70%           
========================================
  Files           75       75           
  Lines          804      804           
  Branches       118      118           
========================================
  Hits           480      480           
  Misses         296      296           
  Partials        28       28

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 ed5df7f...37c5905. Read the comment docs.

chivekrodis
chivekrodis requested changes Jul 4, 2022
View reviewed changes
app/src/componentLibrary/modal/modalLayout/modalLayout.scss Outdated
@@ -75,7 +75,7 @@ $WIDTH-LARGE: 720px;
display: inline-block;
margin-top: 10px;
margin-bottom: 10px;
padding: 32px 48px;
padding: 32px 40px;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you discuss it with UX as you change component from library?
As I know it should stay as is and padding will be the same for all new modal windows

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the task to fix it in new modal window so yes I guess that it's discussed

Copy link
Contributor

Choose a reason for hiding this comment

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

Please discuss it with UX to make sure is everything is as expected

Copy link
Contributor Author

@tr1ble tr1ble Jul 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, we've discussed, we will return 48px

AmsterGet
AmsterGet requested changes Jul 5, 2022
View reviewed changes
app/src/controllers/project/sagas.js Outdated Show resolved Hide resolved
@tr1ble tr1ble requested a review from AmsterGet July 5, 2022 09:23
@AmsterGet AmsterGet merged commit d0b7a39 into develop Jul 5, 2022
@AmsterGet AmsterGet deleted the EPMRPP-78105_Notificaions_preloader branch July 5, 2022 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chivekrodis chivekrodis chivekrodis requested changes

@AmsterGet AmsterGet AmsterGet approved these changes

@Vadim73i Vadim73i Awaiting requested review from Vadim73i

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tr1ble @codecov-commenter @AmsterGet @chivekrodis