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

Angular update #7147

Merged
merged 4 commits into from
Jul 6, 2020
Merged

Angular update #7147

merged 4 commits into from
Jul 6, 2020

Conversation

faho
Copy link
Member

@faho faho commented Jun 22, 2020

Description

As recently discussed on gitter, webconfig uses angular 1.0.8, from 2013.

This ports it to angular 1.8, which entered a 3 year LTS period in 2018. Hey, that's almost new.

As far as I can tell that means angular is dead, so we'd have to figure out something to move to.

So this is more of a stop gap.

(also webconfig is in dire need of some love anyway)

TODOs:

  • [N/A] Changes to fish usage are reflected in user documentation/manpages.
  • [N/A] Tests have been added for regressions fixed
  • User-visible changes noted in CHANGELOG.rst

Note that I have only applied light testing - I clicked through all the tabs and they seemed to work for me.

faho added 3 commits June 21, 2020 22:05
ng-bind-html-unsafe was apparently removed.
This used to use "success", which was our own thing, but which I can't
get working.

So instead we just use ".then", which only passes one object as an
argument that then contains all the other data we use.

This should be enough to complete the port to angular 1.8
@faho faho added this to the fish 3.2.0 milestone Jun 22, 2020
@charego
Copy link
Contributor

charego commented Jun 23, 2020

Clicking the Dracula theme gives this error and breaks the "Customize" button.

Error: "[ngRepeat:dupes] Duplicates in a repeater are not allowed.
Use 'track by' expression to specify unique keys.
Repeater: color in color_array, Duplicate key: string:44475A, Duplicate value: 44475A
https://errors.angularjs.org/1.8.0/ngRepeat/dupes?p0=color%20in%20color_array&p1=string%3A44475A&p2=44475A"

That's the only regression I see from a cursory look through all the tabs.

@faho
Copy link
Member Author

faho commented Jun 23, 2020

That could actually be a bug in the colorscheme? I'm not sure what that "colors" array is good for.

Also is it possible that some of the colorschemes are either broken themselves or have a wrong background color? For "Snow Day" and "Tomorrow" the suggestions aren't visible to me (but my color vision is impaired, so I'm never quite sure).

@zanchey
Copy link
Member

zanchey commented Jun 23, 2020

Also is it possible that some of the colorschemes are either broken themselves or have a wrong background color?

Look ok here, though also note #7117

@charego
Copy link
Contributor

charego commented Jun 24, 2020

Dracula theme has always had a duplicate color (#3723).

I'm not sure what that "colors" array is good for.

These are the selectable colors that show up when you press "Customize". It's fine to have 11 colors in the array, as evidenced by other themes in colorutils.js. I believe the fix is to remove the duplicate.

Also came across an official Dracula fish theme:

It differs from the settings we have. But that's for a different PR.

@ridiculousfish
Copy link
Member

This seems to work OK for me too with light testing.

This was always wrong, but the new(er) angular actually complains
about it.
@faho faho merged commit 213ac15 into fish-shell:master Jul 6, 2020
@faho
Copy link
Member Author

faho commented Jul 6, 2020

Yeah, merged with the Dracula fix, let's see if it shakes anything else out.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants