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

Events, mono-repo discussion #43

Closed
Cacodaimon opened this issue Nov 30, 2014 · 12 comments
Closed

Events, mono-repo discussion #43

Cacodaimon opened this issue Nov 30, 2014 · 12 comments

Comments

@Cacodaimon
Copy link
Owner

Origin Issue: fregante/GhostText#42 (comment)

@Cacodaimon
Copy link
Owner Author

Is the support of keypress events really needed, the W3C deprecated it: http://www.w3.org/TR/DOM-Level-3-Events/#dfn-keypress

Warning!
The keypress event type is defined in this specification for reference and completeness, but this specification deprecates the use of this event type. When in editing contexts, authors can subscribe to the beforeinput event instead.

@fregante
Copy link
Collaborator

It's just a nice-to-have feature since a lot of websites may depend on it for the live preview. The full, ordered list of events that should be fired is

  • keydown
  • keypress
  • input
  • keyup

At the most basic implementation, they should be too hard to implement, it's almost as simple as duplicating this line: https://github.com/Cacodaimon/GhostText-for-Chrome/blob/master/scripts/InputArea/TextArea.ts#L161

GT needs a bit of a attention but its current structure discourages commit(ment)s — TypeScript, multiple repos… but my full-time job doesn't help either.

We should merge it into a single repo like RES does it: https://github.com/honestbleeps/Reddit-Enhancement-Suite

@fregante fregante changed the title GhostText doesn't fire keypress events #42 GhostText doesn't fire all key events Nov 30, 2014
@Cacodaimon
Copy link
Owner Author

At the most basic implementation, they should be too hard to implement, it's almost as simple as duplicating this line: https://github.com/Cacodaimon/GhostText-for-Chrome/blob/master/scripts/InputArea/TextArea.ts#L161

Yes it is rather a fundamental decision about implementing deprecated stuff or putting our little man power elsewhere.

GT needs a bit of a attention but its current structure discourages commit(ment)s — TypeScript, multiple repos… but my full-time job doesn't help either.

Since the InputArea component is shared across the browsers (the Firefox supports the ES6 class syntax and Chrome currently not) we would have two options without TypeScript using ES5 like "classes" or using no OOP here. ES5 classes heavily lacks in maintainability in my opinion. Besides the class based OOP TypeScript provides some other nice features like interfaces and compile type checking, especially the last one speeds up development of new features since the code is more easy to refactor. The only overhead is a little grunt task running in the background during development.

We should merge it into a single repo like RES does it: https://github.com/honestbleeps/Reddit-Enhancement-Suite

I agree with you, the current situation where both repositories having a shared component is suboptimal, I think creating a submodule instead of merging the repositories is a better solution for the InputArea code.

@Cacodaimon
Copy link
Owner Author

I made again some thoughts about this issue, one the one hand many pages might (mis)use this events on the other hand a realistic simulation of keyboard events would be very hard to archive. We would need a keyboard layout database and guess a layout based on the language and the OS… and a diff from each change, send from the server, would be needed for emitting the right sequence of keyboard events.

screenshot - 12152014 - 09 42 41 pm

@fregante
Copy link
Collaborator

No, the events are the ones I showed, they don't depend on the keyboard or language, but just on the browser — the events will be fired by the ST-browser part, so we can customize it per-browser as needed, IF needed.

Really what we're trying to achieve here is activating any live preview/saving that the site might have, so we don't need to generate realistic KeyboardEvents with all those properties. If we want to be cool, "charCode" and "which" are the only ones that might be useful and they are easy to fake.

@fregante
Copy link
Collaborator

About the repo situation, I've read bad things about submodules. Plus, the problem is not just duplicate code, but also issues. Moving issues to the right repo is messy and… where do you place a generic browser-related issue? Firefox or Chrome? Where was it opened? You have to look in all of them to find it (I just had to do that). What about an issue that's only between FF and ST?

One repo would solve those issues. You can tag issues [FF] and [ST] and be done with it

@Cacodaimon
Copy link
Owner Author

No, the events are the ones I showed, they don't depend on the keyboard or language, but just on the browser — the events will be fired by the ST-browser part, so we can customize it per-browser as needed, IF needed.

Really what we're trying to achieve here is activating any live preview/saving that the site might have, so we don't need to generate realistic KeyboardEvents with all those properties. If we want to be cool, "charCode" and "which" are the only ones that might be useful and they are easy to fake.

If it is clear that the events are only fake events I would agree about implementing fake keyboard events.

The only thing to consider is the possibility that a fake events could break a script running on the page, if it expects a shiftKey field which does not exists for example. So I would make this type of events switchable at the options/about page.

About the repo situation, I've read bad things about submodules. Plus, the problem is not just duplicate code, but also issues. Moving issues to the right repo is messy and… where do you place a generic browser-related issue? Firefox or Chrome? Where was it opened? You have to look in all of them to find it (I just had to do that). What about an issue that's only between FF and ST?

Whats about a global repository called GhostText which acts as bug/issue tracker, contains a general Readme.md but without any line of code. And maybe a project homepage. Merging the Chrome, FF, ST and Atom code into one repository could be really problematic as little example Chrome and Atom packages/extensions have a package.json. And the releases from ST, Atom and FF would conflict, a fix in the ST package would require a new release and this release could contain unready code from other Plugins… .

About the repo situation, I've read bad things about submodules. Plus, the problem is not just duplicate code.

I have to admit that I never used submodules but we could give it a try and switch back if it sucks. But this issue has got a low priority in my opinion, especially because of the disastrous situation at Firefox Add-ons submission review system. I'm waiting since the 5. October for a review of the add-on, without this review it is not listed as harmless add-on and is included into the add-on search.

@fregante
Copy link
Collaborator

Problem solved, no need for options:
document.createEvent("KeyboardEvent")


the releases from ST, Atom and FF would conflict, a fix in the ST package would require a new release and this release could contain unready code from other Plugins… .

I hadn't thought of that, but the only software pulling data straight from GitHub is Sublime and we can solve that problem by prefixing Sublime-only updates with "st":

The "tags" key can be true for all valid semantic version tags, or can be a prefix string. Only tags in the form {prefix}{semantic_version} will be selected. https://github.com/wbond/package_control/blob/master/example-repository.json#L265-L268


Chrome and Atom packages/extensions have a package.json

Have you looked at RES' repo? They have browser-specific folders which will automatically get a copy of shared components via gulp. Submodules don't scale well: What happens when we need more than one shared component? Do we create yet more repos?

Sure, GTST doesn't share much with GTCH, but the more stuff we have under one roof, the easier it is to share it across parts, be it explanations, images, instructions, …

The issues-only repo would solve the issues issue (heh), which is better than nothing.


Does Firefox addon situation take development time for us? Aren't we just waiting idly?

@Cacodaimon
Copy link
Owner Author

@Azeirah

Can you provide a example page which uses key events which I can use for testing?

@Azeirah
Copy link

Azeirah commented Feb 4, 2015

here you go

@Cacodaimon
Copy link
Owner Author

I have implemented the keyup event first, it seems that the preview function from the provided page works fine.

@bfred-it
Can you upload a new version to the Chrome web store or shall we implement the other two event first?

@fregante
Copy link
Collaborator

fregante commented Sep 6, 2016

Moved back to fregante/GhostText#42 on the main repo

@fregante fregante closed this as completed Sep 6, 2016
Repository owner locked and limited conversation to collaborators Sep 6, 2016
@fregante fregante changed the title GhostText doesn't fire all key events Events, mono-repo discussion Sep 6, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants