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.

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

Removing inline CSS #13777

Merged
merged 5 commits into from Oct 30, 2017

Conversation

Projects
None yet
2 participants
@skokeers27
Contributor

skokeers27 commented Oct 25, 2017

Before submitting pull request, please check that every commit:

  • Has proper Signed-Off-By
  • Has commit message which describes it
  • Is needed on it's own, if you have just minor fixes to previous commits, you can squash them
  • Any new functionality is covered by tests

skokeers27 added some commits Oct 22, 2017

slightly correct readability
Signed-off-by: Paweł Skokowski <skokowskipawel@icloud.com>
Removed inline css in gis_data_editor.php
Signed-off-by: Paweł Skokowski <skokowskipawel@icloud.com>
Removed inline css in side_menu.twig
Signed-off-by: Paweł Skokowski <skokowskipawel@icloud.com>
@nijel

Thanks for your patch. Besides things I've mentioned inline, the CSS changes should be also implemented in second theme we ship - in themes/original/css/common.css.php

@@ -2503,6 +2501,13 @@
padding: 2px;
}
.clearright {
clear:right;
}

This comment has been minimized.

@nijel

nijel Oct 25, 2017

Member

This is most likely wrong for RTL languages (probably should match floatright).

@nijel

nijel Oct 25, 2017

Member

This is most likely wrong for RTL languages (probably should match floatright).

This comment has been minimized.

@skokeers27

skokeers27 Oct 25, 2017

Contributor

This is original line:
echo '<div class="choice floatright" style="clear:right;">';
Shoudl I left 2 classes, choice and floatright and remove "style="clear:right;" ?

@skokeers27

skokeers27 Oct 25, 2017

Contributor

This is original line:
echo '<div class="choice floatright" style="clear:right;">';
Shoudl I left 2 classes, choice and floatright and remove "style="clear:right;" ?

This comment has been minimized.

@nijel

nijel Oct 25, 2017

Member

Yes, the original code was wrong as well. Hmm, looking at it, this element seems to be hidden all the time anyway, so it's probably better remove anything else from it:

#gis_data_editor .choice {
display: none;
}

@nijel

nijel Oct 25, 2017

Member

Yes, the original code was wrong as well. Hmm, looking at it, this element seems to be hidden all the time anyway, so it's probably better remove anything else from it:

#gis_data_editor .choice {
display: none;
}

This comment has been minimized.

@skokeers27

skokeers27 Oct 25, 2017

Contributor

Ok, I left only choice class.

@skokeers27

skokeers27 Oct 25, 2017

Contributor

Ok, I left only choice class.

Show outdated Hide outdated themes/pmahomme/css/common.css.php Outdated

@nijel nijel changed the title from Iss12262 to Removing inline CSS Oct 25, 2017

added changes to theme original
Signed-off-by: Paweł Skokowski <skokowskipawel@icloud.com>
@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Oct 25, 2017

Codecov Report

Merging #13777 into master will increase coverage by 0.02%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master   #13777      +/-   ##
==========================================
+ Coverage   53.89%   53.91%   +0.02%     
==========================================
  Files         487      487              
  Lines       82336    82307      -29     
==========================================
+ Hits        44374    44379       +5     
+ Misses      37962    37928      -34

codecov bot commented Oct 25, 2017

Codecov Report

Merging #13777 into master will increase coverage by 0.02%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master   #13777      +/-   ##
==========================================
+ Coverage   53.89%   53.91%   +0.02%     
==========================================
  Files         487      487              
  Lines       82336    82307      -29     
==========================================
+ Hits        44374    44379       +5     
+ Misses      37962    37928      -34
added changes to theme original
Signed-off-by: Paweł Skokowski <skokowskipawel@icloud.com>
@skokeers27

changes have been made

@nijel nijel self-assigned this Oct 30, 2017

@nijel nijel merged commit 2191972 into phpmyadmin:master Oct 30, 2017

4 of 6 checks passed

Scrutinizer Analysis: Errored – Tests: passed
Details
codecov/patch 0% of diff hit (target 53.89%)
Details
DCO All commits have a DCO sign-off from the author
codacy/pr Good work! A positive pull request.
Details
codecov/project 53.91% (+0.02%) compared to 125aa9a
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

nijel added a commit that referenced this pull request Oct 30, 2017

Remove clearright class
This element is currently not rendered at all, so avoid adding class for it.

Issue #13777

Signed-off-by: Michal Čihař <michal@cihar.com>

nijel added a commit that referenced this pull request Oct 30, 2017

Remove unused class
Issue #13777

Signed-off-by: Michal Čihař <michal@cihar.com>

nijel added a commit that referenced this pull request Oct 30, 2017

Remove not needed CSS for span elements
These do not have any border by default, so there is no need to remove it.

Issue #13777

Signed-off-by: Michal Čihař <michal@cihar.com>
@nijel

This comment has been minimized.

Show comment
Hide comment
@nijel

nijel Oct 30, 2017

Member

Thanks, I've merged this PR with following fixes:

  • 8109763 Remove not needed CSS for span elements
  • 43cbcf6 Remove unused class
  • 636b22c Remove clearright class
Member

nijel commented Oct 30, 2017

Thanks, I've merged this PR with following fixes:

  • 8109763 Remove not needed CSS for span elements
  • 43cbcf6 Remove unused class
  • 636b22c Remove clearright class
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment