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

[NEW] Unread bars on sidebar (#16853) #16862

Merged
merged 3 commits into from
May 12, 2020

Conversation

juzser
Copy link
Contributor

@juzser juzser commented Mar 12, 2020

Issue number #16853

Screen Shot 2020-03-12 at 23 09 13

  • Change class li.has-alert that never be used, to li.sidebar-item--unread to detect the position of unread message room.
  • Trigger the updateUnreadBars function when receive new unread message.
  • Update theme style for the unread-rooms that is display: none as default.

@CLAassistant
Copy link

CLAassistant commented Mar 12, 2020

CLA assistant check
All committers have signed the CLA.

@juzser juzser force-pushed the fix-unread-bars-on-sidebar branch from e0637f2 to e773c9c Compare March 12, 2020 16:21
@sampaiodiego
Copy link
Member

sampaiodiego commented Mar 12, 2020

thanks a ton for this 🤗 I've been missing these bars for sooooo long now.. @brunosquadros is this something we're considering to have back?

@juzser
Copy link
Contributor Author

juzser commented Mar 13, 2020

Yes, no problem, I'm a fan of you guys.
When I tried to make these bars via custom script, I figured out that you guys had done this.
If there are something wrong, please let me know then I can update the PR.

I see there is a small problem when you resize the window by height (from have scroll on sidebar to scroll disappeared) so the bar is still there.
I'm considering to trigger add hidden class to the bar on resize but I feel it's not really neccessary.

@@ -64,7 +64,9 @@
}

& .unread-rooms {
display: none;
padding: calc(var(--sidebar-small-default-padding) - 8px);
Copy link
Member

Choose a reason for hiding this comment

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

What are those 8 pixels? scrollbar?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the .sidebar-item has 3px padding top and the total height is 34px (base on var(--sidebar-item-height-medium)).
So I put the default padding there plus 8px to equal the sidebar-item

@juzser juzser requested a review from gabriellsh March 14, 2020 14:02
@juzser
Copy link
Contributor Author

juzser commented Mar 21, 2020

So do I need to update anything?

@ggazzo ggazzo changed the base branch from master to develop March 23, 2020 17:46
@ggazzo ggazzo changed the base branch from develop to master March 23, 2020 17:47
@ggazzo ggazzo changed the base branch from master to develop March 23, 2020 17:47
@ggazzo ggazzo force-pushed the fix-unread-bars-on-sidebar branch from 5f0b2ff to 4b7ebdf Compare March 23, 2020 17:49
@ggazzo
Copy link
Member

ggazzo commented Mar 23, 2020

hey, @juzser thanks for your time, I don't we any changes, I will check with design and we gonna merge your pr :) soon as possible

ps: pay attention, you opened a pr to master, you should work always on the develop branch ... thanks again...

@ggazzo ggazzo added this to the 4.0.0 milestone Mar 23, 2020
@juzser
Copy link
Contributor Author

juzser commented Mar 24, 2020

Nice. I see you pick my commit to develop, thank you. 👍🏼
Next time I will work on develop branch.

@engelgabriel engelgabriel modified the milestones: 4.0.0, 3.3.0 May 12, 2020
@ggazzo ggazzo closed this May 12, 2020
@ggazzo ggazzo reopened this May 12, 2020
@ggazzo ggazzo merged commit 960bed4 into RocketChat:develop May 12, 2020
@ggazzo ggazzo changed the title [FIX] Unread bottom, top bars on sidebar (#16853) [NEW] Unread bars on sidebar (#16853) May 12, 2020
ggazzo added a commit that referenced this pull request May 12, 2020
…to remove-get-dom

* 'remove-get-dom' of github.com:RocketChat/Rocket.Chat:
  [FIX] Notification sounds (#17616)
  [FIX] Resolve 'app already exists' error on app update (#17544)
  [NEW] Unread bars on sidebar (#16862)
  [FIX] Relative image path in oembededUrlWidget (#15902)
  Regression: Do not show custom status inside sequential messages (#17613)
gabriellsh added a commit that referenced this pull request May 13, 2020
… integrations

* 'integrations' of github.com:RocketChat/Rocket.Chat:
  [NEW] Add Permissions to deal with Omnichannel visitor past chats history (#17580)
  [NEW] Add permissions to deal with Omnichannel custom fields (#17567)
  [FIX] Livechat iframe allow microphone and camera (#9956)
  [FIX] Do not allow passwords on private channels (#15642)
  [FIX] Mail Messages > Cannot mail own user. (#17625)
  [FIX] remove multiple options from dontAskMeAgain (#17514)
  [FIX] Notification sounds (#17616)
  [FIX] Resolve 'app already exists' error on app update (#17544)
  [NEW] Unread bars on sidebar (#16862)
  [FIX] Relative image path in oembededUrlWidget (#15902)
  Regression: Do not show custom status inside sequential messages (#17613)
@rodrigok rodrigok mentioned this pull request May 28, 2020
@rodrigok rodrigok mentioned this pull request May 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants