-
Notifications
You must be signed in to change notification settings - Fork 17
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
Subscriber Home & View Content #659
Conversation
app/subscriber/public/favicon.png
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to ask Bobbi for a smaller version for tab icon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving out of tno-core for now as changes coming to our tables
@@ -2,17 +2,8 @@ | |||
<html lang="en"> | |||
<head> | |||
<meta charset="utf-8" /> | |||
<link rel="icon" href="%PUBLIC_URL%/favicon.ico" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we be including this with a new icon?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a few places that this was being pulled from, I left it in the manifest.json but we could. Will do another look over to make sure everything is in line
@@ -62,6 +62,7 @@ public IPaged<Content> FindWithDatabase(ContentFilter filter, bool asNoTracking | |||
.Include(c => c.Source) | |||
.Include(c => c.Series) | |||
.Include(c => c.License) | |||
.Include(c => c.TonePoolsManyToMany).ThenInclude(ct => ct.TonePool) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While helpful, I don't think this is going to help. We shouldn't be using this for search from the frontend anymore. We're using elasticsearch. The method below this one.
c09b28d
to
e4c1b14
Compare
Just trying to figure out elastic search, missing some fields and learning the equivalent for .Include() |
@Fosol - being stumped by this - think I will just mark it as ready for review as is. Currently will not show tone icons on list view - but the rest of the code can be reviewed. Might be good for me to switch gears and finish off the filters first. This will need to be merged before my other PR that I am working on. Will figure out including |
</Row> | ||
<Row justifyContent="center" className="date-navigator"> | ||
<FaAngleLeft /> | ||
2023-03-04 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded date?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a placeholder for my next ticket!
PR Includes