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.

Sign up for GitHub

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

Emoji encoding 3123 #3161

Merged
merged 13 commits into from
Aug 7, 2018
Merged

Emoji encoding 3123 #3161

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
278243b
initial setup commit
cheneyshreve Jun 8, 2018
c71b392
Merge remote-tracking branch 'upstream/master'
cheneyshreve Jun 21, 2018
1e08b0b
merge upstream changes
cheneyshreve Jun 23, 2018
953c96e
Merge branch 'master' of https://github.com/publiclab/plots2
cheneyshreve Jun 29, 2018
bcc07ef
Merge remote-tracking branch 'upstream/master'
cheneyshreve Jul 19, 2018
6d294ed
Merge branch 'master' of https://github.com/publiclab/plots2
cheneyshreve Jul 23, 2018
012a920
[WIP] refactoring atwho code, moved primary code to assets/javascript…
cheneyshreve Jul 26, 2018
b0377da
removed script from rich text editor, removed emoji file from public …
cheneyshreve Jul 30, 2018
5ecdc9d
reverting file config/database.yml.sqlite
cheneyshreve Aug 3, 2018
83a0ffb
renamed emoji script to atwho_autocomplete, modified to read var emoj…
cheneyshreve Aug 3, 2018
a3a2514
moved error code to script file
cheneyshreve Aug 4, 2018
ceb6b67
moved error code out of script to form, wrapped it in a function
cheneyshreve Aug 7, 2018
b012269
amended git workflow regarding public/lib/emoji.js
cheneyshreve Aug 7, 2018
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
1 change: 1 addition & 0 deletions app/assets/javascripts/application.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,4 @@
//= require noty_notification.js
//= require jquery-confirm/js/jquery-confirm.js
//= require jquery.atwho
//= require emoji.js
Copy link
Member

Choose a reason for hiding this comment

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

I think I'd prefer not to include the entire emoji.js file here, and to leave it in the /lib/ folder. These files are compiled together, and I think we can treat the emoji file as a static asset, so the system doesn't have to process it (since it's so big!). But we should include comment-autocomplete.js here -- i actually don't see that one in the PR, did you forget to add it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @jywarren I think that I did a poor job naming the emoji script, in that it is named the same as the file that held the emoji json. In this version of the code, emoji.js is the script living in app/assets/javascripts/emoji.js that includes within it the defined emojis (not being read from public, just defined in a var), and deleted the file from public/lib/emoji.js since it would not be needed. I can remove the require statement from application.js.

I'm not sure about the comment-autocomplete.js? I had not done anything with that, when I add it in to application.js I get the error couldn't find file 'comment-autocomplete.js' with type 'application/javascript' so perhaps I need to fetch more recent changes and do a rebase? I believe I have the config/database.yml.sqlite removed properly now so once I clear up the details above, I'll resubmit.

Thanks again for all of your help! And the callout in the community checkin, it is really wonderful to have the opportunity to work with you on this :)

39 changes: 39 additions & 0 deletions app/assets/javascripts/emoji.js
View file
Open in desktop

Large diffs are not rendered by default.

42 changes: 1 addition & 41 deletions app/views/comments/_form.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,51 +64,13 @@
</script>

<%= javascript_include_tag "dragdrop" %>
<script src="/lib/emoji.js" type="text/javascript"></script>
<script src="/assets/emoji.js" type="text/javascript"></script>
<script>
$('#comment-form').bind('ajax:beforeSend', function(event){
$("#text-input").prop('disabled',true)
$("#comment-form .btn-primary").button('loading',true);
});

(function() {

var at_config = {
at: "@",
callbacks: {
remoteFilter: function(query, callback) {
$.getJSON("/api/srch/profiles?srchString=" + query, {}, function(data) {
if (data.hasOwnProperty('items') && data.items.length > 0) {
callback(data.items.map(function(i) { return i.docTitle }));
}
});
}
},
limit: 20
},
hashtags_config = {
at: "#",
callbacks: {
remoteFilter: function(query, callback) {
if (query != ''){
$.post('/tag/suggested/' + query, {}, function(response) {
callback(response.map(function(tagnames){ return tagnames }));
});
}
}
},
limit: 20
},
emojis_config = {
at: ':',
data: Object.keys(emoji).map(function(name){ return {'name': name, 'value': emoji[name]} }),
displayTpl: "<li>${value} ${name}</li>",
insertTpl: ":${name}:",
limit: 100
}

$('textarea#text-input').atwho(at_config).atwho(hashtags_config).atwho(emojis_config);

$('#comment-form').bind('ajax:success', function(e, data, status, xhr){
Copy link
Member

Choose a reason for hiding this comment

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

Can you confirm that the error message here still works without the (function() { around it? Maybe best to add that back in, as it helps the whole thing run /after/ page load is complete?

Copy link
Member

Choose a reason for hiding this comment

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

I think adding it back in may be easier than testing it... i know you can get it to error either by turning off your locally running app OR by submitting a real unicode emoji (not just a :smiley:) -- since we're trying to fix that! But... could be a bit tricky to test.

Copy link
Collaborator Author

@cheneyshreve01 cheneyshreve01 Aug 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @jywarren, I moved the code for error message back into the function athwho_autocomplete script! Quick question --- so the public folder is in .gitignore-- how do we then make sure we're including /lib/emoji.js? I just realized it wasn't getting included when I made that code change above then freaked out momentarily because "emoji " wasn't defined then realized it was because the file is in .gitignore so I had to go add it in again. Thanks again for all of your help!

I'll go ahead and push up changes--we just need to make sure that /lib/emoji.js gets included and it's currently in .gitignore.

$('#text-input').prop('disabled',false);
$('#text-input').val('');
Expand All @@ -127,8 +89,6 @@
$('#comment-form .control-group').append('<span class="help-block ">Error: there was a problem.</span>')
});

})();

function insertTitleSuggestionTemplate() {
var element = $('#text-input');
var currentText = $('#text-input').val().trim();
Expand Down
8 changes: 2 additions & 6 deletions app/views/editor/rich.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,13 +168,13 @@
<% if controller.action_name != "edit" && !current_user.first_time_poster %>
<span id="newOpts" style="display: inline-block;">
<span id="text" style="display: none; color:red;">
Others won't see your post yet
Others won't see your post yet
(<a href="https://publiclab.org/draft-feature" targrt="_blank">?</a>) &nbsp; &nbsp;</span>
<span class="checkbox">
<label>
<input type="checkbox" id="myCheck" onclick="draftCheck()" />
Save as draft
</label> |
</label> |
</span>
<span class="ple-steps-left">2</span>
<span class="hidden-xs"> steps left</span>
Expand All @@ -185,8 +185,6 @@
</div>

</div>


<script>

var editor;
Expand Down Expand Up @@ -294,7 +292,6 @@
}
}
});

})();

$('.legacy-button').click(function onLegacyClick(event) {
Expand All @@ -317,5 +314,4 @@
}
console.log("Draft:", editor.data.draft);
}

</script>
11 changes: 11 additions & 0 deletions config/database.yml.sqlite
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
development:
Copy link
Member

Choose a reason for hiding this comment

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

Hi, this is looking great, but would you mind removing the this file from your PR, as it's unrelated to your changes?

https://stackoverflow.com/questions/215718/reset-or-revert-a-specific-file-to-a-specific-revision-using-git

Thanks!

adapter: sqlite3
database: development.sqlite

production:
adapter: sqlite3
database: production.sqlite

test:
adapter: sqlite3
database: test.sqlite
10 changes: 5 additions & 5 deletions config/initializers/assets.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Rails.application.config.assets.precompile += [
Rails.application.config.assets.precompile += [
'leaflet-blurred-location/dist/Leaflet.BlurredLocation.js',

'advanced_search.js',
'application.js',
'comment_expand.js',
Expand Down Expand Up @@ -30,7 +30,7 @@
'textbox_expand.js',
'users.js',
'wikis.js',

'blog.css',
'comments.css',
'dashboard.css',
Expand All @@ -48,6 +48,6 @@
'user_tags.css.scss',
'wiki.css',
'horsey.js',
'comment-autocomplete'
'comment-autocomplete',
'emoji.js'
]

1 change: 0 additions & 1 deletion public/lib/emoji.js
View file
Open in desktop

This file was deleted.