search

pbaity / rocketchat-dark-mode

An easy user-togglable dark mode for Rocket.Chat
MIT License
369 stars 126 forks source link

3.12.x - Quotes, Pinned messages, Sidebar in panels #125

Closed TBG-FR closed 3 years ago

TBG-FR commented 3 years ago

Edit: Now also fixes #122

Enjoy ;)

pbaity commented 3 years ago

Thanks @TBG-FR , I just merged #123 which introduced another conflict here. If that fix works in 3.12.x for you, we can keep it, otherwise let me know.

TBG-FR commented 3 years ago

I checked, and #123 and my commits are separate things, but git have seen them as conflicts anyway, that should be good now ;)

fdellwing commented 3 years ago

The conflict is that body.dark-mode .rcx-attachment__content .rcx-css-16qhlle is a dead rule, it was replaced in #123 with body.dark-mode .rcx-message-attachment .rcx-attachment__content .rcx-box--full and the !important was removed as it was no longer necessary.

So maybe just move my new rule to the location you commented with /* Attachements content */.

P.S. We should maybe wait with this until 3.12.1 drops, 3.12.0 contains a serious bug with Jitsi and will hopefully be patched really quickly. Afterwards the generated classes would be needed to be replaced again.

TBG-FR commented 3 years ago

The conflict is that body.dark-mode .rcx-attachment__content .rcx-css-16qhlle is a dead rule, it was replaced in #123 with body.dark-mode .rcx-message-attachment .rcx-attachment__content .rcx-box--full and the !important was removed as it was no longer necessary.

So maybe just move my new rule to the location you commented with /* Attachements content */.

P.S. We should maybe wait with this until 3.12.1 drops, 3.12.0 contains a serious bug with Jitsi and will hopefully be patched really quickly. Afterwards the generated classes would be needed to be replaced again.

I'm sorry I thought I placed your new rule here, I should open my eyes more ahah I'll do that, that was the point of my pull of master but looks like I failed !

Yeah I'm also following the Jitsi bug... I'll update the code anyway and we'll see !

TBG-FR commented 3 years ago

Looks like I did it right in fact, if you look at the latest commits, your code is under /* Attachements */ and the code above it is related to quotes ;)

fdellwing commented 3 years ago

Nah I think you are mixing things up. Currently there are two things related to attachements in your PR.

Line 188ff contains the new rule for text attachements Line 203ff contains the rule that #123 removed

TBG-FR commented 3 years ago

Yeah, it's really time to close the computer, didn't see that the old rule was still there 😅 I'll remove the rule line 203 then !

fdellwing commented 3 years ago

Thanks, I'll take a look at your new rules at work tomorrow.

fdellwing commented 3 years ago

I'm not completely happy with the PR.

I would like to add the following rule for blockquotes:

body.dark-mode blockquote.rcx-attachment__details .rcx-box--full {
  color: var(--primary-font-color);
}

Also you would be very nice if you change --color-white to --primary-font-color in line 190.

TBG-FR commented 3 years ago

I'll modify the rule line 190, no issue.

However, I'd say we should use color: var(--secondary-font-color); for quotes, as it's what is done in the regular RocketChat theme : quoted content isn't as "light" as normal content (they're using the rule color: var(--rcx-color-foreground-info, #6c727a) !important;)

image

I'll put that, but if you don't agree, let me know @fdellwing !

fdellwing commented 3 years ago

In that case just ignore my request and use the color RC intends to. I'll create a fork and change it there so that is looks good for us.

We already overwrite the --color-darkest value to a much lighter grey, so I guess thats a good point to fork.

TBG-FR commented 3 years ago

In that case just ignore my request and use the color RC intends to. I'll create a fork and change it there so that is looks good for us.

We already overwrite the --color-darkest value to a much lighter grey, so I guess thats a good point to fork.

Alright ! Still, sorry for the inconvenience...

@pbaity I think we're good on this one ;)

fdellwing commented 3 years ago

Did you check the PR against the new 3.12.1 published today?

TBG-FR commented 3 years ago

I'm checking it as we speak, everything seems to work well : quotes, modals, sidebars, ...

pbaity commented 3 years ago

@TBG-FR this looks great to me, just tested it out in 3.12.1 myself. If you're ready for it to me merged, I'll merge it.

TBG-FR commented 3 years ago

@pbaity I'm ready ;)

pbaity commented 3 years ago

Thanks to you and @fdellwing both for the great work on this PR!

fdellwing commented 3 years ago

Thanks for the work @TBG-FR , I just created a new branch in my fork to track our custom changes.