Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Updated colorpicker css styles to remove extra pixels #1948

Merged
merged 5 commits into from
Aug 31, 2018

Conversation

Blackbaud-TrevorBurch
Copy link
Member

Resolves #1941

@codecov-io
Copy link

codecov-io commented Aug 28, 2018

Codecov Report

Merging #1948 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1948   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         424     424           
  Lines        8968    8968           
  Branches     1326    1326           
======================================
  Hits         8968    8968

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 5cd9ee3...8108219. Read the comment docs.

@Blackbaud-AlexKingman
Copy link
Contributor

@Blackbaud-TrevorBurch your work definitely fixes the problem, but lets chat about the larger CSS structure of the colorpicker here for a sec. The markup looks something like this:

<sky-colorpicker>
    <div class="sky-input-group">
        <input> // the visual representation of the picker
        <sky-dropdown> // the hidden popup & the thing you actually click on!
            ... the dropdown button and some other dropdown stuff
        </sky-dropdown>
        <div *ngIf="toggleReset">
            <button> // reset
        </div>
    </div>
</sky-colorpicker>

The big trick to this whole thing is making it appear like the input is the clickable widget, when in fact, its not. We do that with some textbook positioning of a hidden element on top of the input to intercept the click. However, it looks like when this was originally written, we put the positioning code on children elements of the dropdown. In this case, sky-colorpicker .sky-dropdown-button, which is a child element of the actual dropdown. Then, we're doing some re-adjusting of its siblings: .sky-colorpicker-reset-button --> left: -25px; (which you found and correctly changed).

What I'm proposing is this: Setting positioning on deep children elements seems risky and takes more code. Would we have more control by setting styles on the parent instead of its children individually? For example, remove all the positioning rules to sky-colorpicker .sky-dropdown-button and ..sky-colorpicker-reset-button. Then, add the following to sky-colorpicker sky-dropdown --> position:absolute. This should always work because we have a sky-input-group in our template with position:relative. The absolutely-positioned sky-dropdown will know to always set its origin to that parent.

@Blackbaud-TrevorBurch
Copy link
Member Author

@Blackbaud-AlexKingman I made some changes here to go with what you suggested. I also removed a /deep/ in favor of an ::ng-deep

Copy link
Contributor

@Blackbaud-AlexKingman Blackbaud-AlexKingman left a comment

Choose a reason for hiding this comment

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

This looks great. Nice work! 🚢 ahoy!

@Blackbaud-TrevorBurch Blackbaud-TrevorBurch merged commit 3ab6c6d into master Aug 31, 2018
@Blackbaud-TrevorBurch Blackbaud-TrevorBurch deleted the 1941-hidden-colorpicker-reset-padding branch August 31, 2018 14:33
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.

3 participants