Change to non-uglified emby-webcomponents #341
Conversation
cvium
force-pushed
the
unminify_webcomponents
branch
from
f8f743d to
3e51b1f
Compare
| if (e.ctrlKey) { | ||
| return; | ||
| } | ||
| if (!!e.shiftKey) { |
There was a problem hiding this comment.
Why shift is handled differently? Either all should be bool-forced or none, IMHO.
| if (chr.length === 1) { | ||
| currentDisplayTextContainer = this.options.itemsContainer; | ||
| onAlphanumericKeyPress(e, chr); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function alphanumeric(value) { | ||
| var letterNumber = /^[0-9a-zA-Z]+$/; |
There was a problem hiding this comment.
so this basically screws non-English languages... isn't there a js builtin to be used instead of this alphanumeric thing?
| } | ||
|
|
||
| var alpanumericShortcutTimeout; |
There was a problem hiding this comment.
shoudn't it be initialized upon definition?
| } | ||
|
|
||
| function selectByShortcutValue(container, value) { | ||
|
|
There was a problem hiding this comment.
this empty line seems unnecessary
| var focusElem; | ||
| "#" === value && (focusElem = container.querySelector("*[data-prefix]")), focusElem || (focusElem = container.querySelector("*[data-prefix^='" + value + "']")), focusElem && focusManager.focus(focusElem) | ||
| if (value === '#') { | ||
|
|
There was a problem hiding this comment.
This, too. And I think there would be more, but I won't nitpick further.
|
|
||
| profile.MaxStreamingBitrate = bitrateSetting; | ||
| profile.MaxStaticBitrate = 100000000; | ||
| profile.MusicStreamingTranscodingBitrate = Math.min(bitrateSetting, 192000); |
There was a problem hiding this comment.
Looks strange to artificially limit audio bitrate to 192kbps
| null; | ||
|
|
||
| if (options.maxVideoWidth) { | ||
| maxVideoWidth = options.maxVideoWidth; |
There was a problem hiding this comment.
shouldn't we check if options.maxVideoWidth is less or equal to maxVideoWidth if latter is not null?..
| VideoCodec: 'vpx', | ||
| Context: 'Streaming', | ||
| Protocol: 'http', | ||
| // If audio transcoding is needed, limit channels to number of physical audio channels |
There was a problem hiding this comment.
It is strange that this channel forcing is not applied for all transcodings, only for some
| }); | ||
| } | ||
|
|
||
| if (browser.chromecast) { |
There was a problem hiding this comment.
Is it also true for Chromecast Ultra?..
| @@ -1,384 +1,331 @@ | |||
| .card, | |||
There was a problem hiding this comment.
Disregard this comment, it is for me to bookmark where I paused reading.
|
These "conflicts" were introduced as a result of me changing the default themes to the dark theme for all UI elements. I've had similar merge "conflicts" in my own branches. Unlikely to need any actual work done to merge but if so ping me and I can make sure its merging without breaking my changes. |
cvium
changed the title
joshuaboniface
changed the title
|
What's left to clean up here? |
|
Also blocked by #424 |
|
Opened issue to track review comments: jellyfin/jellyfin-web#10 Opened new PR on web repo: jellyfin/jellyfin-web#1 |
Until the webui can be properly restructured (separate repo or whatever), any work on the UI is currently hampered by the main repo only having the uglyfied JS.
I've changed emby-webcomponents to the proper 1.5.323 from https://github.com/jellyfin/emby-webcomponents. It should be the one bundled with 3.5.2 (changes in 1.5.324 are not included in the main repo at least).
See discussion here: jellyfin-archive/emby-webcomponents#1