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

Directory network selector #2219

Merged
merged 6 commits into from Sep 16, 2016
Merged

Directory network selector #2219

merged 6 commits into from Sep 16, 2016

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Sep 15, 2016

Adds the network selector to filter down the room directory by what network the room is on. Currently works by having a hardcoded list of regexes that it tests all the room aliases against, awaiting a better interface on the server.

Copy link
Member

@ara4n ara4n left a comment

Choose a reason for hiding this comment

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

lgtm otherwise

@@ -32,6 +32,13 @@ var sanitizeHtml = require('sanitize-html');

linkifyMatrix(linkify);

const NETWORK_PATTERNS = {
'gitter': /#gitter_.*/,
Copy link
Member

Choose a reason for hiding this comment

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

surely these should all be hardcoded to :matrix.org as the domain?


this.networkIcons = {
'matrix:matrix_org': '//matrix.org/favicon.ico',
'irc:freenode': '//matrix.org/_matrix/media/v1/download/matrix.org/DHLHpDDgWNNejFmrewvwEAHX',
Copy link
Member

Choose a reason for hiding this comment

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

hah :P it might be a bit less cute to just include the icons in vector though otherwise these are adding unnecessary 3rd party dependencies and vector is meant to be usable without 'net access.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh , definitely: I was going to change it when we had actual icons for them, which I thought you'd probably want input on.

@ara4n
Copy link
Member

ara4n commented Sep 15, 2016

i'd expect to see config.sample.json being updated? (and possibly committed as config.json, so that clients pick up the configured networks by default?)

otherwise LGTM

@dbkr
Copy link
Member Author

dbkr commented Sep 15, 2016

Can do - I didn't put it in the sample sinc eit's hopefully very short term , i.e this will be unnecessary once the server sends the network info: same for not defaulting it (plus you won't get any networks unless you're pulling in matrix.org's directory which has now been turned off).

@ara4n
Copy link
Member

ara4n commented Sep 15, 2016

hm, ok. let's at least put an example in sample.json. othewrise lgtm.

(ie. every alias on the matrix.org HS, so currently everything, since we don't pull in any other directories)
@ara4n
Copy link
Member

ara4n commented Sep 15, 2016

i still think that we should be putting the actual :matrix.org bridges in the example config, so that folks can use them (when they query the matrix.org HS). otherwise, lgtm.

@ara4n
Copy link
Member

ara4n commented Sep 16, 2016

dave: lgtm, please merge when safe

@dbkr dbkr merged commit 135c22c into develop Sep 16, 2016
@t3chguy t3chguy deleted the dbkr/directory_network_selector branch May 12, 2022 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants