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

Add ability to customize CSS of 'content' div #795

Merged
merged 5 commits into from
Sep 25, 2019
Merged

Add ability to customize CSS of 'content' div #795

merged 5 commits into from
Sep 25, 2019

Conversation

acicovic
Copy link
Collaborator

Attention: Credit for this PR goes to @PhilippGrashoff who is the real contributor of this code. For more info see #793.

This adds the ability to customize the CSS classes which are used for the <div class="img content atk-dialog-content>". E.g. add 'scrolling' for scrolling content.

Also fixes Method addScrolling, which added the 'scrolling' class to the main div instead of the content div.

If you think defining an extra method for it is overkill, we could go the same way as $headerCss is.

@acicovic acicovic mentioned this pull request Aug 18, 2019
Closed
@codecov
Copy link

codecov bot commented Aug 18, 2019

Codecov Report

Merging #795 into develop will decrease coverage by 0.06%.
The diff coverage is 25%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop     #795      +/-   ##
=============================================
- Coverage      76.54%   76.47%   -0.07%     
- Complexity      2251     2255       +4     
=============================================
  Files            119      119              
  Lines           5337     5344       +7     
=============================================
+ Hits            4085     4087       +2     
- Misses          1252     1257       +5
Impacted Files Coverage Δ Complexity Δ
src/Modal.php 85.1% <25%> (-4.55%) 42 <3> (+4)

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 8b5b7ce...18db033. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Aug 18, 2019 •
edited
Loading

Codecov Report

Merging #795 into develop will decrease coverage by 0.06%.
The diff coverage is 25%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop     #795      +/-   ##
=============================================
- Coverage      76.54%   76.47%   -0.07%     
- Complexity      2251     2255       +4     
=============================================
  Files            119      119              
  Lines           5337     5344       +7     
=============================================
+ Hits            4085     4087       +2     
- Misses          1252     1257       +5
Impacted Files Coverage Δ Complexity Δ
src/Modal.php 85.1% <25%> (-4.55%) 42 <3> (+4)

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 8b5b7ce...18db033. Read the comment docs.

@romaninsh romaninsh added the Documentation 📚 Undocumented features label Aug 22, 2019
@romaninsh
Copy link
Member

romaninsh commented Aug 22, 2019 •
edited by PhilippGrashoff
Loading

Looks awesome but :

  • add some docs (at least mention!)
  • add tests

romaninsh
romaninsh approved these changes Aug 22, 2019
View reviewed changes
Copy link
Member

@romaninsh romaninsh left a comment

Choose a reason for hiding this comment

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

see my comment.

@codecov-io
Copy link

codecov-io commented Aug 22, 2019 •
edited by codecov bot
Loading

Codecov Report

Merging #795 into develop will decrease coverage by 0.3%.
The diff coverage is 77.77%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop     #795      +/-   ##
=============================================
- Coverage      76.54%   76.23%   -0.31%     
- Complexity      2251     2291      +40     
=============================================
  Files            119      119              
  Lines           5337     5404      +67     
=============================================
+ Hits            4085     4120      +35     
- Misses          1252     1284      +32
Impacted Files Coverage Δ Complexity Δ
src/Modal.php 91.48% <77.77%> (+1.83%) 42 <3> (+4) ⬆️
src/jsVueService.php 80% <0%> (-20%) 5% <0%> (ø)
src/TableColumn/FilterModel/TypeString.php 33.33% <0%> (-4.77%) 8% <0%> (+1%)
src/Form.php 73.33% <0%> (-1.33%) 68% <0%> (+2%)
src/App.php 82.51% <0%> (-0.31%) 129% <0%> (-2%)
src/FormField/MultiLine.php 36.92% <0%> (+4.67%) 140% <0%> (+35%) ⬆️

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 8b5b7ce...93a7108. Read the comment docs.

@PhilippGrashoff
Copy link
Collaborator

Am lost writing a test for this :) Do I get it correctly that:

  1. in a demo page, use that feature
  2. write a test that uses this demo page
    ?

@romaninsh
Copy link
Member

romaninsh commented Aug 29, 2019 •
edited
Loading

Simply add to modal demo page, and it should be fine. I think it will be enough if you add another "modal" to existing page , it should affect coverage.

@romaninsh
Copy link
Member

@PhilippGrashoff check out why modal is not opening , please...

@romaninsh
Copy link
Member

romaninsh commented Sep 19, 2019 •
edited
Loading

@acicovic - are you in ATK contributor team? if yes - you can push branch directly into this repo.. (contact me if you need it)

abbadon1334
abbadon1334 requested changes Sep 19, 2019
View reviewed changes
template/semantic-ui/modal.html Outdated
@@ -1,6 +1,6 @@
<{_element}div{/} id="{$_id}" class="{_ui}ui {/} {$class} {_class}{/}" style="{$style}" {$attributes}><i class="{$close}"></i>
<div class="{$headerCss}">{$title}</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

i think this must be $headerCSS

template/semantic-ui/modal.html Outdated
@@ -1,6 +1,6 @@
<{_element}div{/} id="{$_id}" class="{_ui}ui {/} {$class} {_class}{/}" style="{$style}" {$attributes}><i class="{$close}"></i>
<div class="{$headerCss}">{$title}</div>
<div class="img content atk-dialog-content">{$Content}</div>
<div class="{$contentCss}">{$Content}</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

i think this must be $contentCSS

@acicovic
Copy link
Collaborator Author

@romaninsh If this doesn't get merged automatically, I'll do it after requested changes and review from @PhilippGrashoff.

romaninsh
romaninsh approved these changes Sep 25, 2019
View reviewed changes
Copy link
Member

@romaninsh romaninsh left a comment

Choose a reason for hiding this comment

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

ok

@romaninsh romaninsh merged commit 7fa6837 into atk4:develop Sep 25, 2019
@acicovic acicovic deleted the feature/customize-modal-content-css branch October 7, 2019 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abbadon1334 abbadon1334 abbadon1334 requested changes

@romaninsh romaninsh romaninsh approved these changes

@PhilippGrashoff PhilippGrashoff Awaiting requested review from PhilippGrashoff

Assignees
No one assigned
Labels
Documentation 📚 Undocumented features
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@acicovic @romaninsh @codecov-io @PhilippGrashoff @abbadon1334