Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Add context menu to TagTile #1743

Merged
merged 3 commits into from
Feb 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions src/actions/TagOrderActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,51 @@ TagOrderActions.moveTag = function(matrixClient, tag, destinationIx) {
});
};

/**
* Creates an action thunk that will do an asynchronous request to
* label a tag as removed in im.vector.web.tag_ordering account data.
*
* The reason this is implemented with new state `removedTags` is that
* we incrementally and initially populate `tags` with groups that
* have been joined. If we remove a group from `tags`, it will just
* get added (as it looks like a group we've recently joined).
*
* NB: If we ever support adding of tags (which is planned), we should
* take special care to remove the tag from `removedTags` when we add
* it.
*
* @param {MatrixClient} matrixClient the matrix client to set the
* account data on.
* @param {string} tag the tag to remove.
* @returns {function} an action thunk that will dispatch actions
* indicating the status of the request.
* @see asyncAction
*/
TagOrderActions.removeTag = function(matrixClient, tag) {
// Don't change tags, just removedTags
const tags = TagOrderStore.getOrderedTags();
const removedTags = TagOrderStore.getRemovedTagsAccountData() || [];

if (removedTags.includes(tag)) {
// Return a thunk that doesn't do anything, we don't even need
// an asynchronous action here, the tag is already removed.
return () => {};
}

removedTags.push(tag);
Copy link
Member

Choose a reason for hiding this comment

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

Could you do all this computation (ie. all the function up until this point) in the async action? That way if you have two in flight at once, you don't lose an update.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really, we need to calculate removedTags here for the optimistic update.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see - ok, so having traced through how all the async actions and actions creators work again, the contents of the store gets updated as soon as the event dispatch gets around the event loop (ie. once the pending lands), so the window where you could lose the update is fairly minimal. (Actually I think its the same array object that gets updated each time anyway so possibly it doesn't race at all...)

I guess this is OK then.


const storeId = TagOrderStore.getStoreId();

return asyncAction('TagOrderActions.removeTag', () => {
Analytics.trackEvent('TagOrderActions', 'removeTag');
return matrixClient.setAccountData(
'im.vector.web.tag_ordering',
{tags, removedTags, _storeId: storeId},
);
}, () => {
// For an optimistic update
return {removedTags};
});
};

export default TagOrderActions;
35 changes: 35 additions & 0 deletions src/components/views/elements/TagTile.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { MatrixClient } from 'matrix-js-sdk';
import sdk from '../../../index';
import dis from '../../../dispatcher';
import { isOnlyCtrlOrCmdIgnoreShiftKeyEvent } from '../../../Keyboard';
import ContextualMenu from '../../structures/ContextualMenu';

import FlairStore from '../../../stores/FlairStore';

Expand Down Expand Up @@ -81,6 +82,35 @@ export default React.createClass({
});
},

onContextButtonClick: function(e) {
e.preventDefault();
e.stopPropagation();

// Hide the (...) immediately
this.setState({ hover: false });

const TagTileContextMenu = sdk.getComponent('context_menus.TagTileContextMenu');
const elementRect = e.target.getBoundingClientRect();

// The window X and Y offsets are to adjust position when zoomed in to page
const x = elementRect.right + window.pageXOffset + 3;
const chevronOffset = 12;
let y = (elementRect.top + (elementRect.height / 2) + window.pageYOffset);
y = y - (chevronOffset + 8); // where 8 is half the height of the chevron

const self = this;
ContextualMenu.createMenu(TagTileContextMenu, {
chevronOffset: chevronOffset,
left: x,
top: y,
tag: this.props.tag,
onFinished: function() {
self.setState({ menuDisplayed: false });
},
});
this.setState({ menuDisplayed: true });
},

onMouseOver: function() {
this.setState({hover: true});
},
Expand Down Expand Up @@ -109,10 +139,15 @@ export default React.createClass({
const tip = this.state.hover ?
<RoomTooltip className="mx_TagTile_tooltip" label={name} /> :
<div />;
const contextButton = this.state.hover || this.state.menuDisplayed ?
<div className="mx_TagTile_context_button" onClick={this.onContextButtonClick}>
{ "\u00B7\u00B7\u00B7" }
</div> : <div />;
return <AccessibleButton className={className} onClick={this.onClick}>
<div className="mx_TagTile_avatar" onMouseOver={this.onMouseOver} onMouseOut={this.onMouseOut}>
<BaseAvatar name={name} url={httpUrl} width={avatarHeight} height={avatarHeight} />
{ tip }
{ contextButton }
</div>
</AccessibleButton>;
},
Expand Down
2 changes: 1 addition & 1 deletion src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -500,8 +500,8 @@
"Download %(text)s": "Download %(text)s",
"Invalid file%(extra)s": "Invalid file%(extra)s",
"Error decrypting image": "Error decrypting image",
"Image '%(Body)s' cannot be displayed.": "Image '%(Body)s' cannot be displayed.",
"This image cannot be displayed.": "This image cannot be displayed.",
"Image '%(Body)s' cannot be displayed.": "Image '%(Body)s' cannot be displayed.",
"Error decrypting video": "Error decrypting video",
"%(senderDisplayName)s changed the avatar for %(roomName)s": "%(senderDisplayName)s changed the avatar for %(roomName)s",
"%(senderDisplayName)s removed the room avatar.": "%(senderDisplayName)s removed the room avatar.",
Expand Down
20 changes: 18 additions & 2 deletions src/stores/TagOrderStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ class TagOrderStore extends Store {
const tagOrderingEventContent = tagOrderingEvent ? tagOrderingEvent.getContent() : {};
this._setState({
orderedTagsAccountData: tagOrderingEventContent.tags || null,
removedTagsAccountData: tagOrderingEventContent.removedTags || null,
hasSynced: true,
});
this._updateOrderedTags();
Expand All @@ -70,6 +71,7 @@ class TagOrderStore extends Store {

this._setState({
orderedTagsAccountData: payload.event_content ? payload.event_content.tags : null,
removedTagsAccountData: payload.event_content ? payload.event_content.removedTags : null,
});
this._updateOrderedTags();
break;
Expand All @@ -90,6 +92,14 @@ class TagOrderStore extends Store {
});
break;
}
case 'TagOrderActions.removeTag.pending': {
// Optimistic update of a removed tag
this._setState({
removedTagsAccountData: payload.request.removedTags,
});
this._updateOrderedTags();
break;
}
case 'select_tag': {
let newTags = [];
// Shift-click semantics
Expand Down Expand Up @@ -165,13 +175,15 @@ class TagOrderStore extends Store {
_mergeGroupsAndTags() {
const groupIds = this._state.joinedGroupIds || [];
const tags = this._state.orderedTagsAccountData || [];
const removedTags = new Set(this._state.removedTagsAccountData || []);


const tagsToKeep = tags.filter(
(t) => t[0] !== '+' || groupIds.includes(t),
(t) => (t[0] !== '+' || groupIds.includes(t)) && !removedTags.has(t),
);

const groupIdsToAdd = groupIds.filter(
(groupId) => !tags.includes(groupId),
(groupId) => !tags.includes(groupId) && !removedTags.has(groupId),
);

return tagsToKeep.concat(groupIdsToAdd);
Expand All @@ -181,6 +193,10 @@ class TagOrderStore extends Store {
return this._state.orderedTags;
}

getRemovedTagsAccountData() {
return this._state.removedTagsAccountData;
}

getStoreId() {
// Generate a random ID to prevent this store from clobbering its
// state with redundant remote echos.
Expand Down