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.

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

[Dimmer] Added support for using dimmers only partially on top,center,bottom #186

Merged
merged 5 commits into from
Oct 24, 2018

Conversation

lubber-de
Copy link
Member

@lubber-de lubber-de commented Oct 18, 2018

Description

This adds the possibility to use any dimmer only partially and limited only to the overall height of its content and not over the whole to be covered area. This can be used for example as a caption on any element of choice by simply using
top dimmer, center dimmer or bottom dimmer

Works also very well with transitions 'fade up' or 'fade down'

Testcase

Static without Javascript (just add active to the dimmer as usual)

<div class="ui medium image">
  <div class="ui active top dimmer">
    <div class="content">
      <h3>Look!</h3>
      ui top dimmer
    </div>
  </div>
  <img src="assets/images/avatar/nan.jpg">
</div>

Animation triggered via Javascript

<div class="ui medium image">
  <div class="ui center dimmer">
    <div class="content">
      <h3>Look!</h3>
      ui center dimmer
    </div>
  </div>
  <img src="assets/images/avatar/nan.jpg">
</div>
$('.center.dimmer').dimmer({
    transition: 'fade down',
    on: 'hover'
});

Screenshot

image

Closes

Semantic-Org/Semantic-UI#3459
Semantic-Org/Semantic-UI#6621

@lubber-de lubber-de self-assigned this Oct 18, 2018
@lubber-de lubber-de added type/feat Any feature requests or improvements lang/css Anything involving CSS state/awaiting-reviews Pull requests which are waiting for reviews state/awaiting-docs Pull requests which need doc changes/additions labels Oct 18, 2018
Copy link
Contributor

@prudho prudho left a comment

Choose a reason for hiding this comment

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

Is it just me or center caption dimmer looks blurred compared to others ?

@lubber-de
Copy link
Member Author

@prudho Thanks..., that's the old nasty webkit bug. Fixed by latest commit.

@ColinFrick
Copy link
Member

I'm conflicted with naming this .caption as the content and container of the dimmer can be arbitrary: https://jsfiddle.net/Korinu/L053zgmu/
Why not use something like .partly, .partially or something similar?

@lubber-de
Copy link
Member Author

Why not use something like .partly, .partially or something similar?

I am totally open to this. 🙂 @prudho @hammy2899 Your opinion?

@prudho
Copy link
Contributor

prudho commented Oct 19, 2018

caption doesn't bother me, French translations feels pretty close to the purpose (légende or sous-titre eg. subtitle).

@y0hami
Copy link
Member

y0hami commented Oct 19, 2018

IMHO I think partially makes the most sense when using a dimmer because to me caption seems like it would be its own component.

@prudho
Copy link
Contributor

prudho commented Oct 19, 2018

@hammy2899 is the most english of us 😄, so i'll follow his advice.

@lubber-de
Copy link
Member Author

Then i am going to change it to "partially center" instead of "center partially" @hammy2899 Agreed?

@lubber-de
Copy link
Member Author

lubber-de commented Oct 19, 2018

We could also completely omit partially but simply use top dimmer, center dimmer bottom dimmer by using explicit contains selectors this would not interfere with something like ui bottom aligned dimmer

https://jsfiddle.net/3xsbL8gk/

@lubber-de lubber-de changed the title [Dimmer] Added support for a caption (partly dimmer) [Dimmer] Added support for using dimmers only partially on top,center,bottom Oct 19, 2018
@lubber-de
Copy link
Member Author

Removed the need for any additional className. Much cleaner. I don't know why i haven't done it this way right from the start 🤔

Copy link
Member

@ColinFrick ColinFrick left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@ColinFrick ColinFrick left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@ColinFrick ColinFrick left a comment

Choose a reason for hiding this comment

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

LGTM

@lubber-de lubber-de removed the state/awaiting-docs Pull requests which need doc changes/additions label Oct 24, 2018
@lubber-de
Copy link
Member Author

lubber-de commented Oct 24, 2018

Docs here
Just @hammy2899 and/or @prudho need to approve this also, even if @ColinFrick approved this 3 times already 🤣

@y0hami y0hami merged commit e85b8e5 into fomantic:beta Oct 24, 2018
@lubber-de lubber-de deleted the caption_dimmer branch October 25, 2018 07:19
@lubber-de lubber-de added state/has-docs A issue/PR which requires documentation changes and has the corresponding PR open in the docs repo and removed state/awaiting-reviews Pull requests which are waiting for reviews labels Oct 25, 2018
@lubber-de lubber-de added this to the 2.6.4 milestone Nov 5, 2018
@y0hami y0hami mentioned this pull request Nov 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang/css Anything involving CSS state/has-docs A issue/PR which requires documentation changes and has the corresponding PR open in the docs repo type/feat Any feature requests or improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants