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

Enhance UI for User-Configured Pinning Service in Tabula #248

Merged
merged 11 commits into from
Sep 14, 2023

Conversation

juliopavila
Copy link
Collaborator

@juliopavila juliopavila commented Aug 6, 2023

Description:

Closes #238 #246

This PR introduces several improvements and fixes around the handling of pinning services in Tabula. It aims to guide users more effectively based on their configuration and make the UI more informative and user-friendly.

Major Changes:

  1. Introduced Pinning Configuration Modal:

    • Allows users to select between using our pinning service, setting up a custom pinning service, or posting the article directly on the chain.
  2. Added Pinning Configuration and Pinning Selection Components:

    • These new components handle the logic and UI for configuring pinning services.
  3. Refactored EditorRichText Component:

    • Improved the handling of pinning status and added conditions for image selection based on pinning configuration.
  4. Code Cleanup and UI Enhancements:

    • Removed redundant code and enhanced the UI with more informative and user-friendly components.
  5. Refactored Wallet View and useIpfs Hooks:

    • Simplified the logic for navigation and content uploading based on the selected pinning service.
  6. Updated usePoster Hook:

    • Modified the createArticle and updateArticle methods to adapt to the new pinning service configurations.

Detailed File Changes:

  • Added new components:

    • PinningConfiguration.tsx
    • PinningSelection.tsx
  • Modified existing components:

    • EditorRichText.tsx
    • UploadFile.tsx
    • ArticleSidebar.tsx
    • CreateArticleView.tsx
    • WalletView.tsx
    • useIpfs.ts
    • usePoster.ts
    • pinning.ts
  • Removed old code from the following files:

    • EditorInlineText.tsx
    • PublicationAvatar.tsx
    • turndown.ts
    • PinningAlert.tsx
    • SetupIpfsView.tsx
  • Modified package.json:

    • Added a new script for starting the application with an OpenSSL legacy provider.

Testing Performed

To ensure that the pinning configuration works as expected and that users can interact with the various cases, the following tests have been performed:

Test Case 1:

  • Action: Upload the content of an article directly on-chain.
  • Expected Result: The article's content is stored directly on the chain. result
  • Action: Edit the publication, switching to the Tabula Pinning Service.
  • Expected Result: The hash of the content should be stored in the article instead of the direct content. result

Test Case 2:

  • Action: Upload the article content using Tabula Pinning Service.
  • Expected Result: The hash of the content is stored in the article. result
  • Action: Edit the publication, switching to directly on-chain.
  • Expected Result: The hash should be replaced by the content directly on the chain. result

Test Case 3:

  • Action: Upload the content of an article directly on-chain.
  • Expected Result: The article's content is stored directly on the chain. result
  • Action: Edit the publication, switching to Custom Pinning Service.
  • Expected Result: The hash of the content should be stored in the article instead of the direct content. result

Screenshots:

Pinning Configuration Modal
image
image

Disabled Image component of the editor when the pinning service is not set up
Disabled Image

Context:

These changes are aimed at guiding users appropriately based on their configuration and making the UI more informative and user-friendly. The implemented solution has been discussed here.

Please refer to the linked threads for further context.

@vercel
Copy link

vercel bot commented Aug 6, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
tabula ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 12, 2023 5:18pm

Copy link
Contributor

@asgeir-s asgeir-s left a comment

Choose a reason for hiding this comment

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

@juliopavila and @cedricwaxwing; here are my comments:

  1. I thought we decided to run a pinning service ourselves (Gnosis Guild) for the first option instead of relying on a public one. So that the first option would be something like "Use the Pinning Service hosted by us...".
  2. Shouldn't the "warning" be displayed in a modal that will pop up when the user clicks "Publish", if they have not made the choice before? Also, I am not sure if it's appropriate to use a warning here (it's just a step of the prosses, as long as it's not selected before and no custom pinning service is configurated).
  3. Shouldn't each of the options be selectable, like with a radio button or something? Because they need to pick one of them.
  4. Also, we should probably store the selection in locale storage so we do not prompt them again.

Copy link
Contributor

@asgeir-s asgeir-s left a comment

Choose a reason for hiding this comment

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

This is how I think it should work:

I think we should remove the "Update Store Configuration" button from the drop-down.
The modal should only be displayed when the user clicks "Publish", and no pinning service option is previously selected.

The "Choose store option" modal should probably be renamed to "How do you want to make your articles available?" or something like that.

If the user wants to edit their pinning service setup later, it should happen under "Setup Pinning Service", as was the case before this PR (just with the two new options added to the drop-down many, as described below).

We should add the options "None (post content on-chain)" and "Public Tablua pinning service" to the pinning service drop-down in the "Setup Pinning Service"-modal.

The button in the modal should probably say "Continue", and it should probably not use a yellow box around the text in the modal.

If the user selects our pinning service option or "Post article on-chain", we must automatically configure the pinning service when the user clicks continue (this is the same thing as what is displayed in the dropdown in the "Setup Pinning Service" modal). If the user selects the set up a pinning service alternative and clicks continue, we must display the "Setup Pinning Service" modal.

But anyway, I think @cedricwaxwing needs to take a look at this.

@cedricwaxwing
Copy link
Collaborator

My thinking is that the pinning modal shouldn't ever really be shown - either when the user first connects the wallet from the landing page or when they go to publish (even when they have the default setting selected). I think we could potentially include a small callout (there should be an option to dismiss this callout permanently) on the article editing page that mentions this and directs them to a pinning page where they can adjust their choice, but in general, I think it's safe to assume that most people will be ok with the default settings, and anyone who understands pinning will immediately notice the callout and figure out how to set up their own from the prompt (which should link to the main pinning settings UI). We can include "(default)" and "(recommended)" next to the default and custom pinning options respectively.

Something else I noticed is that the API spec link is not visibly a link, and the clickable area also expands past the text.
image

@juliopavila
Copy link
Collaborator Author

@manboy-eth @cedricwaxwing The last push contains the following improvements:

  • Remove "Update Store Configuration" Button: The button will be removed from the drop-down menu to simplify the options.
  • Modal Display Condition: The modal should only appear when the user clicks "Publish" and hasn't selected a pinning service option.
  • Modal Renaming: The modal's title should be more descriptive, like "How do you want to make your articles available?"
  • Editing Pinning Service: If users want to change their pinning service later, they should do it under "Setup Pinning Service", as before.
  • Button Text and Style: The button in the modal should say "Continue" and should not have a yellow box around the text.
  • Automatic Configuration: If the user selects either of the new options and clicks "Continue", the system should automatically configure the pinning service.
  • Avoid Pinning Modal: Pinning modal should not be shown by default. Instead, users should be directed to a pinning page where they can adjust their settings.
  • Default and Recommended Labels: Labels like "(default)" and "(recommended)" could be added next to the pinning options to guide user choice.
  • API Spec Link Issue: The API spec link is not visibly a link, and the clickable area extends beyond the text. This needs to be fixed.
  • Refactor the UI of the Pinning Service configuration

Copy link
Collaborator

@cedricwaxwing cedricwaxwing left a comment

Choose a reason for hiding this comment

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

Just finished exploring the new changes.

Great work @juliopavila - this is such a big improvement in terms of the UX. 🙌

Copy link
Contributor

@asgeir-s asgeir-s left a comment

Choose a reason for hiding this comment

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

Nice work @juliopavila 🚀

@juliopavila juliopavila merged commit 571bbb9 into tabula-v.2.1 Sep 14, 2023
4 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Sep 14, 2023
@juliopavila juliopavila deleted the issue-#238 branch May 28, 2024 20:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants