Skip to content

Cleanup contributions view #735

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.

Sign up for GitHub

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

Jump to bottom
Merged
phylum merged 3 commits into from
Sep 25, 2020
Merged

Cleanup contributions view #735

phylum merged 3 commits into from
Sep 25, 2020

Conversation

@thestephenmarshall
Copy link
Collaborator

@thestephenmarshall thestephenmarshall commented Sep 24, 2020 •
edited
Loading

localhost_3000_contributions(iPad)
localhost_3000_contributions(iPad) (1)
localhost_3000_contributions(iPad) (2)

Why

Updates the contributions view layout for browse (tile|list|map) buttons, tiles and list view items. Relates to #282.

Pre-Merge Checklist

All these boxes should be checked off before any pull request is merged!

  • All new features have been described in the pull request
  • Security & accesibility have been considered
  • All outstanding questions and concerns have been resolved
  • Any next steps that seem like a good ideas have been created as issues for future discussion & implementation
  • High quality tests have been added, or an explanation has been given why the features cannot be tested
  • New features have been documented, and the code is understandable and well commented
  • Entry added to CHANGELOG.md if appropriate

What

  • Update browse view buttons
  • Update Tile view
  • Update list view

How

Update layout based on contributions mocks

Testing

Navigate to the /contributions view and visually inspect

@thestephenmarshall thestephenmarshall self-assigned this Sep 25, 2020
@thestephenmarshall thestephenmarshall marked this pull request as ready for review September 25, 2020 12:50
thestephenmarshall
thestephenmarshall commented Sep 25, 2020
View reviewed changes
app/assets/stylesheets/bulma_and_buefy.scss
.subnav {
.navbar-item {
&.is-active {
color: $navbar-item-hover-color;
Copy link
Collaborator Author

@thestephenmarshall thestephenmarshall Sep 25, 2020

Choose a reason for hiding this comment

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

By default, the navbar (when active) remains #000. This ensures that anchor tags in a subnav which are active are visible.

@svileshina
Copy link
Collaborator

svileshina commented Sep 25, 2020

if you have the time for this:
Screen Shot 2020-09-25 at 1 21 50 PM

But also, not a big deal and we can address it at another point.

@phylum phylum merged commit 70662d8 into main Sep 25, 2020
@phylum phylum deleted the feature/contributions-cleanup branch September 25, 2020 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@svileshina svileshina svileshina approved these changes

@solebared solebared Awaiting requested review from solebared

@maebeale maebeale Awaiting requested review from maebeale

Assignees

@thestephenmarshall thestephenmarshall

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@thestephenmarshall @svileshina @phylum