Search code, repositories, users, issues, pull requests...

Copy link

Member

@cardoso cardoso commented Dec 14, 2018 •

edited

Loading

@RocketChat/ios

Had to move some stuff around to share markdown code with the extension

Partially closes #1563
Blocked by RocketChat/Rocket.Chat#12956


 Remove markdown from push notifications

372d229

@cardoso cardoso added the 👍 improvement label

@cardoso cardoso added this to the 3.3.0 milestone

houndci-bot

houndci-bot reviewed

Rocket.Chat/Helpers/MessageTextFontAttributesColors.swift Outdated Show resolved Hide resolved

Rocket.Chat/Extensions/UIColor/UIColorMentions.swift Outdated Show resolved Hide resolved

Rocket.Chat.NotificationServiceExtension/NotificationService.swift Outdated Show resolved Hide resolved


 Fix linter issues

f0f4002

rafaelks

rafaelks reviewed

Rocket.Chat/Managers/MarkdownManager.swift Outdated

parser.strikeAttributes = [NSAttributedString.Key.strikethroughStyle.rawValue: NSNumber(value: NSUnderlineStyle.single.rawValue)]

parser.linkAttributes = [NSAttributedString.Key.foregroundColor.rawValue: UIColor.darkGray]

#if !EXCLUDE_MARKDOWN_DOWNLOAD

Copy link

Contributor

@rafaelks rafaelks Dec 14, 2018

Choose a reason for hiding this comment

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

I love the idea of using this kind of configuration per scheme, but what about something "pattern", like this?

ROCKETCHAT_MARKDOWN_DOWNLOAD_MEDIA

This way we:

Make sure the name is actually managed by us;
It's a positive checking, not negative, will be like this:

#if ROCKETCHAT_MARKDOWN_DOWNLOAD_MEDIA
...
#endif

I think it brings more clarity to the code... what do you think?

Copy link

Member Author

Choose a reason for hiding this comment

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

Agreed with namespacing the flag
If positive then we have to remember to add it for every build configuration in every target :/ this is very specific for the Notification Service Extension to avoid including all our download related classes.

rafaelks

rafaelks reviewed

Rocket.Chat/Extensions/UIColor/UIColorCSSColorsExtension.swift

]

}

// swiftlint:enable all

Copy link

Contributor

@rafaelks rafaelks Dec 14, 2018

Choose a reason for hiding this comment

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

👀

Copy link

Member Author

Choose a reason for hiding this comment

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

I decided to treat this file as an external library. It has tons of single letter variables that prevent compiling with our linter.

Should I rewrite it? What do you think?

@cardoso cardoso added the 🚫 blocked: other issue label

cardoso added 3 commits

December 14, 2018 16:27


 Merge branch 'develop' into imp/remove_md_pn

c4294b7


 Better naming for exclude markdown download flag

ef05427


 Merge branch 'imp/remove_md_pn' of https://github.com/RocketChat/Rock...

fb12908

...et.Chat.iOS into imp/remove_md_pn

houndci-bot

houndci-bot reviewed

Dec 21, 2018

Rocket.Chat/External/RCEmojiKit/Emojione+Transform.swift Outdated Show resolved Hide resolved

@cardoso cardoso changed the title ~~(削除) [BLOCKED][IMPROVEMENT] Remove markdown from push notifications (削除ここまで)~~ (追記) [BLOCKED][IMPROVEMENT] Remove markdown & show emojis in push notifications (追記ここまで)

Dec 21, 2018

@cardoso cardoso changed the title ~~(削除) [BLOCKED][IMPROVEMENT] Remove markdown & show emojis in push notifications (削除ここまで)~~ (追記) [BLOCKED][IMPROVEMENT] Remove markdown & parse emojis in push notifications (追記ここまで)

Dec 21, 2018