-
Notifications
You must be signed in to change notification settings - Fork 2
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
implements URO-211 part B - link creation #221
Open
eapearson
wants to merge
16
commits into
main
Choose a base branch
from
URO-211
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Not an effort to fully correct the typing for JSON-RPC 2.0, just this case which interfered with using it.
Added because existing loader does not allow for additional content to entertain the user
used to inform the user how long their linking session has left before expiration
ensures consistent formatting of a user's ORCID Id, compliant with ORCID branding requirements
A common error is simply one thrown w/in the ui, with some extra context to help the user.
either moved to local components if used once, or removed, if not necessary, or moved to common/style.module.scss
it was only used in this component
ported over from kbase-ui and modified to fit; can be replaced by RTK later
part 1, mostly a placeholder to receive the redirect; second installment will add the service call to fetch the error definition.
changes made around the linking process and improvements.
sorry for the import reordering diff, it had to be done at some point (!) and otherwise the only functional changes are in the orcidlink routes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
[ DRAFT ] - still need to create the PR description, observe the GHA outcome, and respond to code quality alerts. Then I'll request a review. Not sure I'll have time to respond to all requests for change before the EOD Friday, but we can try.
The purpose of this PR is to introduce the functionality and user interface to support the creation of an ORCID Link. It builds upon previous work to establish the basic UI components and testing support.
It does not include the following functionality:
The latter two will be implemented in a follow-up PR (or at least branch).
The main area of concern in this PR is probably the approach to interacting with the orcidlink service during linking. Because the linking process is fleeting, and doesn’t really affect application state, a non-RTK client is used in some places. This is also because I didn’t have time to understand RTK well enough to implement this style of query. Anyway, the code for this was extracted from kbase-ui, modified to fit, and (new) tests written.
Another area of note is the usage of “compound components”, in which a stack of related components implements the navigation entry points. It is similar to MVC, in that there is a controller, and there is a view. The controller is responsible for fetching data and providing action functions to the view, and the view is primarily responsible for the ui (although the controller may display loading and error messages). In some cases there is also an “entrypoint” component which is responsible for ensuring that route parameters are populated. The latter is required by various factors, such as the optional nature of route params and search params and the decoupling of app state and component flow (e.g. getting auth state even when behind the Authed component).
I imagine that some of these implementation decisions could be solved by other means.