Skip to content

[NEW] Global announcement#8461

Closed
mrsimpson wants to merge 142 commits into
RocketChat:developfrom
assistify:core/#8446-global-announcement
Closed

[NEW] Global announcement#8461
mrsimpson wants to merge 142 commits into
RocketChat:developfrom
assistify:core/#8446-global-announcement

Conversation

@mrsimpson

@mrsimpson mrsimpson commented Oct 11, 2017

Copy link
Copy Markdown
Contributor

@RocketChat/core Forked off 0.59.0-rc12

Motivation

Closes RocketChat/feature-requests#606

  • Configurable global announcement via settings
  • Uses theme "attention-color" for styling
  • Appears on Login, Home and Channel-Screens
  • Can be confirmed if a user is logged-in
  • Button floats which allows long and short texts

Remarks

Don't complain to me with respect to the default colors!
I applied all of my non-existent css-skills to it - if you've got suggestions, shoot!

This is how it looks like

Short texts

short texts

Longer texts

long texts

Configuration

config

"Mobile"

On very small displays (mobile), it hides the burger button. However, I do not think that this is a problem. If you (RC) consider it wrong, kindly help me out on the CSS ;)
mobile

Feedback (and of course merging) appreciated

rodrigok and others added 30 commits August 24, 2017 22:00
…ignment

[FIX] message actions over unread bar
* Fix hyperlink style on sidebar footer

* fix a selector
…ow-rocketDebug

[FIX] Window exception when parsing Markdown on server
…e-api-notifications

[FIX] Remove break change in Realtime API
[FIX] Fix google play logo on repo README
…irst-load

[FIX] Show leader on first load
[DOCS] Add native mobile app links into README and update button images
…placeholders

[FIX] Fix placeholders in account profile
[FIX] Document README.md. Drupal repo out of date
[FIX] status and active room colors on sidebar
…list

[FIX] Fix the status on the members list
…de-render

[FIX] Markdown being rendered in code tags
rodrigok and others added 10 commits October 9, 2017 09:35
[FIX] Sidebar item menu position in RTL
…port

[FIX] Slack import failing and not being able to be restarted
…reconnect

Fix: Missing LDAP reconnect setting
…internal-log-level

Fix: Missing LDAP option to show internal logs
- Configurable global announcement via settings
- Uses theme "attention-color" for styling (don't complain to me with respect to the default colors!)
- Appears on Login, Home and Channel-Screens
- Can be confirmed if a user is logged-in
- Button floats which allows long and short texts
@geekgonecrazy

Copy link
Copy Markdown
Contributor

Ok this is awesome!! I like this UI much better then our current per channel announcement as well.

@rocketchat/core this would also be useful on our community server for announcements. I'm sure useful to many others

@mrsimpson

Copy link
Copy Markdown
Contributor Author

@geekgonecrazy glad you like it. One thing which isn’t amazing is that the announcement re-appears after confirmation once the server restarts (e. G. Due to a new deployment). The reason for that is that I’m using the lastUpdated impaired to the readConfirmation timestamp in order to determine whether the announcement shall appear. Currently, lastUpdated is getting updated on startup though. I consider this a bug, wdyt?

@marceloschmidt

Copy link
Copy Markdown
Member

Finally!!! This is a long-time needed feature. Thanks so much for implementing it.

@geekgonecrazy

Copy link
Copy Markdown
Contributor

@mrsimpson I'd agree most would consider this a bug. But not a show stopper for this PR in my opinion

mrsimpson and others added 4 commits November 1, 2017 10:42
…l-announcement

# Conflicts:
#	.snapcraft/snapcraft.yaml
#	README.md
#	packages/rocketchat-api/server/v1/channels.js
#	packages/rocketchat-api/server/v1/groups.js
#	packages/rocketchat-google-vision/.npm/package/npm-shrinkwrap.json
#	packages/rocketchat-i18n/i18n/de.i18n.json
#	packages/rocketchat-i18n/i18n/en.i18n.json
#	packages/rocketchat-ldap/server/ldap.js
#	packages/rocketchat-ldap/server/sync.js
#	packages/rocketchat-markdown/tests/client.tests.js
#	packages/rocketchat-theme/client/imports/components/messages.css
#	packages/rocketchat-theme/client/imports/components/popover.css
#	packages/rocketchat-theme/client/imports/components/sidebar/sidebar.css
#	packages/rocketchat-theme/client/imports/general/base.css
#	packages/rocketchat-theme/client/imports/general/base_old.css
#	packages/rocketchat-theme/client/imports/general/variables.css
#	packages/rocketchat-ui/client/lib/chatMessages.js
#	packages/rocketchat-ui/client/views/app/popover.js
#	packages/rocketchat-ui/client/views/app/room.js
#	server/publications/room.js
No idea why git didn't overwrite them earlier.

@mrsimpson mrsimpson left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just merged the latest develop` and manually removed differences which remained to to other branch-off.

return this.update(query, update);
}

confirmGlobalAnnouncementRead(_id) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this actually is the only part which really changed in this file. The rest is only pretty printing.

}

/* on Login-screen */
.wrapper .global-announcement {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

maybe, dedicated classes should be used. However, I wanted to modify as least as possible

const settingGlobalAnnouncement = RocketChat.models.Settings.findOne({_id: 'Layout_Global_Announcement'});
const user = RocketChat.models.Users.findOne({_id: Meteor.userId()});

return !!user.globalAnnouncementRead && (user.globalAnnouncementRead > settingGlobalAnnouncement._updatedAt);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@geekgonecrazy comparing with _updatedAt has the flaw of making the annoucement re-appear on server startup.
If the settings were not upserted on add(), but only if the value set differs, this would not be the case anymore.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this is ok for merging. Maybe we should take a look at modding that behavior though..

@geekgonecrazy

Copy link
Copy Markdown
Contributor

@karlprieb a good bit of UI stuff here. Can you take a quick peek? :)

@geekgonecrazy

Copy link
Copy Markdown
Contributor

We had a PR come along after yours: #9778 I think does the same. I'm guessing when we went to create we didn't search for announcement. Also ours doesn't reach as far as yours....

@rodrigok @karlprieb what do we want to do?

@karlprieb

Copy link
Copy Markdown
Contributor

I think we should close this PR and extend #9778

@geekgonecrazy

Copy link
Copy Markdown
Contributor

@mrsimpson as mentioned above we implemented one to handle version alert. It's very similiar but is a banner across the entire top.

Maybe we could extend that?

@mrsimpson

Copy link
Copy Markdown
Contributor Author

I’m always happy to delete Code I wrote - reduces maintenance effort. It shouldn’t be a big deal to extend the general alert banner.
Diff:

  • some admin UI to maintain the text
  • show the general alert on login screen
  • handle multiple alerts

If @karlprieb was handling this, that would be great.

Sent with GitHawk

@mrsimpson mrsimpson closed this Apr 21, 2018
@mrsimpson mrsimpson deleted the core/#8446-global-announcement branch December 7, 2020 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[NEW] Global announcement

9 participants