Closed unhammer closed 3 years ago
@sushain97 it only shows on modes that actually have preferences defined, see https://apertium.trigram.no/index.nob.html?dir=nno-nob#translation , so should be minimal impact, but maybe I've made some horrible decision that should be changed before merging :)
@unhammer haven't had a chance to look at this closely yet but you'll want to rebase to incorporate Tino's style changes.
Took a look at https://apertium.trigram.no/index.eng.html?dir=nob-nno#translation though and my immediate reaction is that the style menu opening moves all the content below which is super distracting. Could we make it so that it opens on top of the lower content? Should be possible with a little CSS.
@sushain97 rebased – how does https://apertium.trigram.no/index.eng.html?dir=nob-nno#translation look now?
To me, the UX of this button is confusing. Why not having something like the language selector button (that toggles on click, instead of hover), to give a consistent UX?
because I suck at web dev? :)
Oooh, I like that way more.
The only minor nits I'd have are:
Need a little spacing below the prefs button.
when it's closed you mean?
@sushain97 try now
Anything to be done before this can be merged now?
Looks good to me
https://905-28576409-gh.circle-artifacts.com/0/build/index.html doesn't load anymore? Some errors in the console. Does not bode well :D Looks like an APy 404?
On a general note, I'm somewhat FUD-ful about landing a new feature on master
since I hope to switch this entire repo over to https://github.com/sushain97/apertium-html-tools-v2 Very Soon. I'm happy to take a crack at porting this feature if you're not up for it. From that perspective, it might make sense to freeze master
for new features, especially if there are still TODOs here (like the localization bit). At the same time, tags exist so maybe I'm being silly?
That'd be great if you could try porting, if you're doing that stuff anyway. Shouldn't be very complicated – it's just about getting the list of preferences from apy, and sending them comma-separated on /translate – most of the code changes here are UI-fiddling. Last time I tried combining react + typescript my computer kept running out of ram on webpacking and I gave up after wasting two days trying to make it compile; I'd rather not try that again before I upgrade (you'd think 16G was enough for js development :-/).
Last time I tried combining react + typescript my computer kept running out of ram on webpacking and I gave up after wasting two days trying to make it compile; I'd rather not try that again before I upgrade (you'd think 16G was enough for js development :-/).
Ha! I feel your pain. I deal with a webpack build that requires up to 24 GiB + 32 cores and still takes ~20 minutes.
But, knowing that, I decided not to use Webpack here. More details in https://github.com/sushain97/apertium-html-tools-v2#dependencies but tldr is that an uncached build takes < 5 seconds on my Macbook (excluding all the APy fetches). I don't know what the peak RSS is but I'd be surprised if it were over a couple hundred megs.
I'm interested in knowing whether that works for you. There's also a docker setup if you don't want to get Node + Yarn installed on your machine.
I fixed that TODO, so it should be functionally complete.
The circleci build artifact failure is expected – the new apy endpoint /pairprefs
isn't in a release yet.
I fixed that TODO, so it should be functionally complete.
Excellent. Thanks! That does make me feel much better.
The circleci build artifact failure is expected – the new apy endpoint
/pairprefs
isn't in a release yet.
Huh, that means that master
wouldn't be safe for deployment to apertium.org
or beta.apertium.org
, right? That seems not great. If we hide this behind a config flag that would fix it?
The circleci build artifact failure is expected – the new apy endpoint /pairprefs isn't in a release yet.
Huh, that means that master wouldn't be safe for deployment to apertium.org or beta.apertium.org, right? That seems not great. If we hide this behind a config flag that would fix it?
/pairprefs should work on beta.apertium.org
but give a 404 on apertium.org
.
I noticed now we were curling without -f in Makefile, so it didn't fail on 404's. I've fixed that for the curls of build/js/listrequests.js
, and made the /pairprefs step optional so it builds with both latest release and master of apy.
Thanks! Rebased and squashed, but github requires a new approval :( mind hitting the button again @sushain97 ?
$ git diff 0babbfe dc270a0
diff --git a/Makefile b/Makefile
index db22b0a..450fb62 100644
--- a/Makefile
+++ b/Makefile
@@ -248,6 +248,19 @@ server:
exo-open "http://localhost:8082" || open "http://localhost:8082"
( cd build && python3 -m http.server 8082 )
+### Tests ###
+# may have to run this first:
+# $ pipenv install --dev
+# $ yarn install
+check:
+ yarn run eslint --config assets/js/.eslintrc.json assets/js/*.js
+ yarn run flow-coverage-report --config flow-coverage.json
+ yarn run flow check
+ yarn run sass-lint --config assets/css/.sass-lint.yml --verbose --no-exit --max-warnings 0
+ yarn run htmlhint --config .htmlhintrc.json *.html
+ find assets/strings -type f -name '*.json' -exec yarn run json --validate -f {} \;
+ pipenv run flake8 --config tools/.flake8.ini *.py **/**.py **/**/*.py
+ cd assets/strings && make && git diff --exit-code .
### Clean ###
clean:
and sending preferences in the form expected by apy/translate
Preferences are refreshed on changing translation languages.
Localisation of the preference list happens on page load and locale change.
cf. https://github.com/apertium/apertium/issues/118