Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Issue 1498 - Adding entity fields #1526
Issue 1498 - Adding entity fields #1526
Changes from 4 commits
3caa62d
5298573
ea63f98
fe12de1
9bf0742
45a4fbd
25ec932
96e0b97
b51b556
0cee6b4
af635d6
1987b37
2679fa8
33875e2
e838c6c
d0289e9
1b4870a
6b43e43
4d64152
3501604
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Check the order of these on the settings screen. I ended up putting this in spot 2, left column, and moved the Feeds settings to top of the middle column. But I don't stare at this screen much, so it's a guess.
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.
could these
h3
s belabel
s instead? the smaller type should look good as welli would keep it
Entity Name
but change:Entity Contact Email
->Administrative Contact Email
Entity Contact Phone
->Public Phone Number
Entity Location
->Your Service Area
Entity Website
->Website Address
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.
"Your" felt redundant on looking at it, like it could be in all labels. This OK, or you think the "your" belongs?
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.
that's good! maybe make them
label
s though, if they're not alreadyThere 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.
do we need to add feedback email here? the idea is that they should pass through from the imported service entity
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.
i think we need some intro text here potentially like "This meeting listing is provided by:"
let's not show the email address - this is more for administrative purposes
the feedback email form should either be hidden and we add something here, or it should send to the entity's feedback_email addresses if present
minor: could we use the
$meeting->data_source
pattern for this and the other vars?minor: i would like to steer away from using
<br>
tags - could this be Ps or a UL?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.
Adding "This meeting listing is provided by:" only concern it will show up everywhere if we display for local and imported meetings, and it just adds that one more line to what your eye is scanning for
Look correct to you? Should it be smaller?
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.
Honestly this raises questions I want to talk about, cause it'll be a mess typing it out. Don't feel equipped with enough history on this tool to answer them myself.
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.
hm, yeah, i'm not so sure. perhaps less is more?
my reason for wanting to add it is that we want to make it clear what this section is - some users might get confused and think it is meeting information.
the purpose of this section is to be like this section in the app:
if we could replicate the headline style for the other sections for the entity name, and make the url and phone buttons like in this pic:
we might try the
This meeting listing is provided by:
above the headline in a<small>
?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.
here's what it currently looks like in this PR for TSML UI:
not that it needs to match per se but just for context