Skip to content

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

Merged
romaninsh merged 5 commits intoatk4:developfrom
acicovic:feature/customize-modal-content-css
Sep 25, 2019
Merged

Add ability to customize CSS of 'content' div#795
romaninsh merged 5 commits intoatk4:developfrom
acicovic:feature/customize-modal-content-css

Conversation

@acicovic
Copy link
Copy Markdown
Collaborator

@acicovic acicovic commented Aug 18, 2019

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
Copy Markdown

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
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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
Copy Markdown
Collaborator

PhilippGrashoff commented Aug 22, 2019

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
Copy Markdown
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
Copy Markdown
Member

romaninsh commented Aug 29, 2019

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

@romaninsh
Copy link
Copy Markdown
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
Copy Markdown
Collaborator

@abbadon1334 abbadon1334 Sep 19, 2019

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
<{_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
Copy Markdown
Collaborator

@abbadon1334 abbadon1334 Sep 19, 2019

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
Copy Markdown
Collaborator Author

acicovic commented Sep 19, 2019

@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
Copy Markdown
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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@acicovic @romaninsh @codecov-io @PhilippGrashoff @abbadon1334