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

Improve UX of aspect chooser #1552

Merged
merged 8 commits into from
Nov 1, 2021

Conversation

carlinmack
Copy link
Collaborator

@carlinmack carlinmack commented Jul 28, 2021

50% of closing #1322 (the other half is the addition of a link to /curation)

Description

Please include a summary of the change, relevant motivation and context. If possible and applicable, include before and after screenshots and a URL where the changes can be seen.

The format of this mirrors the URL so hopefully users will figure out with use

image

With chooser open

image

On pages with a single aspect

image

On pages with a sub aspect

image

Caveats

Please list anything which has been left out of this PR or which should be considered before this PR is accepted
Check any of the following which apply:

I am not incorporating the "CiTO" subaspect as I aim to handle that in a different way

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
    • I have made corresponding changes to the documentation
  • This change requires new dependencies (please list)

From the Bootstrap documentation:

Dropdowns are built on a third party library, Popper.js, which provides dynamic positioning and viewport detection. Be sure to include popper.min.js before Bootstrap’s JavaScript or use bootstrap.bundle.min.js / bootstrap.bundle.js which contains Popper.js. Popper.js isn’t used to position dropdowns in navbars though as dynamic positioning isn’t required.

We could use bootstrap.bundle.min.js instead of popper.min.js which is added by this PR?

If you make changes to the python code

  • my code passes tox check, you can receive warnings about tests, documentation or both

Testing

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Checked the links on pages with one, multiple and sub-aspects

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have not used code from external sources without attribution
  • I have considered accessibility in my implementation
    • I am assuming that the Bootstrap dropdown is accessible
  • There are no remaining debug statements (print, console.log, ...)

@carlinmack carlinmack changed the title Consistent nav 1322 Improve UX of aspect chooser Jul 28, 2021
@carlinmack
Copy link
Collaborator Author

Added support for subaspects

image

@egonw egonw added ready for merge label to flag a pull request ready for decision by the gatekeeper for merge into master and removed ready for review labels Nov 1, 2021
@egonw
Copy link
Collaborator

egonw commented Nov 1, 2021

Looks good to me. @fnielsen, please use Rebase and Merge instead of Merge pull request. Rebase just tested successfully and here are the tox results:

  flake8: commands succeeded
  pydocstyle: commands succeeded
  py39: commands succeeded
  congratulations :)

@fnielsen fnielsen merged commit fdb54d8 into WDscholia:master Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for merge label to flag a pull request ready for decision by the gatekeeper for merge into master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants