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

Styling for notes #7258

Merged
merged 1 commit into from
Sep 2, 2022
Merged

Styling for notes #7258

merged 1 commit into from
Sep 2, 2022

Conversation

lucascumsille
Copy link
Contributor

Relevant issue(s)

Related to #7240 and #7243

What does this do?

Added styling to notes when visiting:

  • Authority page
  • Request page
  • New request page

Screenshot 2022-08-31 at 10 10 10

@garethrees
Copy link
Member

I'll leave @gbp to look at the code on this one. Would be good to add screenshots for all pages where these notes apply (on the free and pro UIs) so I can +1 without needing to set it all up myself 🙏

@lucascumsille
Copy link
Contributor Author

lucascumsille commented Aug 31, 2022

I'll leave @gbp to look at the code on this one. Would be good to add screenshots for all pages where these notes apply (on the free and pro UIs) so I can +1 without needing to set it all up myself 🙏

Sure no problem, @garethrees

WDTK theme

Screenshot 2022-08-31 at 11 49 06

Screenshot 2022-08-31 at 11 49 16

Screenshot 2022-08-31 at 11 49 24

None

Screenshot 2022-08-31 at 12 01 29

Screenshot 2022-08-31 at 12 02 05

Pro

Screenshot 2022-08-31 at 12 08 24

I just noticed that the element #request_header_text, has some styling. For what I can see is not being used anywhere else. Do you want me to get rid of it?

Screenshot 2022-08-31 at 12 11 37

@lucascumsille
Copy link
Contributor Author

One thing to notice @garethrees
On the request page, because I couldn't see the notes I took and added this fragment
<% unless @batch %> <% if @info_request.public_body.has_notes? %> <div id="request_header_text" class="request_header_text"> <%= render_notes(@info_request.public_body.notes) %> </div> <% end %> <% end %>
into the new.html.erb file so I can see the notes. I got rid of that after talking with @gbp, so he will will include a in a future commit.

@garethrees
Copy link
Member

I just noticed that the element #request_header_text, has some styling. For what I can see is not being used anywhere else. Do you want me to get rid of it?

I don't think we want the blue notes rendering in the green box, so if you think that's what needs to happen, sure!

Copy link
Member

@garethrees garethrees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me!

Need to resolve the #request_header_text issue before merge, but happy to approve based on what's presented and leave the final signoff/merge to @gbp

@lucascumsille
Copy link
Contributor Author

@garethrees I just created a PR on the alavetelitheme repo for the #request_header_text element
Let me know if there is a more straight forward way to handle this =)

@garethrees
Copy link
Member

@garethrees I just created a PR on the alavetelitheme repo for the #request_header_text element Let me know if there is a more straight forward way to handle this =)

I don't know off hand; something for you and @gbp to figure out 🕵️ !

@gbp gbp force-pushed the 387-notes-by-tag branch from e2e41e1 to a30dd58 Compare September 1, 2022 10:28
This style applies to
- New request page
- Authority page
- Request page
@gbp gbp changed the base branch from 387-notes-by-tag to develop September 2, 2022 10:35
Copy link
Member

@gbp gbp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy with your changes here and in alaveteli theme. Nice one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants