-
Notifications
You must be signed in to change notification settings - Fork 24
Alter attribute input for mentions #720
Alter attribute input for mentions #720
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #720 +/- ##
==========================================
- Coverage 90.04% 89.87% -0.18%
==========================================
Files 113 83 -30
Lines 16059 14603 -1456
Branches 565 0 -565
==========================================
- Hits 14460 13124 -1336
+ Misses 1579 1479 -100
+ Partials 20 0 -20
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
140d97a
to
e2b8344
Compare
729f5ba
to
b1ca983
Compare
#[wasm_bindgen_test] | ||
fn a_with_attributes() { | ||
roundtrip( | ||
r#"<a contenteditable="false" data-mention-type="user" style="something" href="http://example.com">a user mention</a>"#, | ||
); | ||
} | ||
|
||
#[wasm_bindgen_test] | ||
fn a_with_bad_attribute() { | ||
let html = r#"<a invalidattribute="true" href="http://example.com">a user mention</a>"#; | ||
let dom = HtmlParser::default().parse::<Utf16String>(html).unwrap(); | ||
assert_eq!( | ||
dom.to_string(), | ||
r#"<a href="http://example.com">a user mention</a>"# | ||
); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No longer good tests, as we don't have links with attributes
case 'insertAtRoomSuggestion': { | ||
if (suggestion && isAtRoomSuggestionEvent(event)) { | ||
const { attributes } = event.data; | ||
// we need to track data-mention-type in element web, ensure we do not pass | ||
// it in as rust model can handle this automatically | ||
if (attributes.has('data-mention-type')) { | ||
attributes.delete('data-mention-type'); | ||
} | ||
return action( | ||
composerModel.insert_at_room_mention_at_suggestion( | ||
suggestion, | ||
attributes, | ||
), | ||
'insert_at_room_mention_at_suggestion', | ||
); | ||
} | ||
break; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This replaces the if text === ...
check from before to completely separate mention and at mention handling
export type AllowedMentionAttributes = Map< | ||
'style' | 'data-mention-type', | ||
string | ||
>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change the type to be a Map
, which fixes another open issue and we also make it less permissive in terms of what it accepts.
@@ -108,5 +108,6 @@ export function useWysiwyg(wysiwygProps?: WysiwygProps) { | |||
traceAction: testUtilities.traceAction, | |||
}, | |||
suggestion: memoisedMappedSuggestion, | |||
messageContent: composerModel?.get_content_as_message_html() ?? null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exposing the ability to use the rust model to output message format html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to see this change happen!
@@ -831,8 +831,8 @@ mod js { | |||
self.current_path.push(DomNodeKind::Link); | |||
|
|||
let mut attributes = vec![]; | |||
let valid_attributes = | |||
["contenteditable", "data-mention-type", "style"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
2ed5d77
to
42300cd
Compare
SonarCloud Quality Gate failed. |
This PR:
attribute
argument from calls to mention functions #721, but still require removal from the mobile side of the codebasedata-mention-type
automaticdata-mention-type
attribute in web, and changes the type for attributes to a map