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

Added rescan page translations and removed dev mode tag #838

Conversation

nischal-shetty2
Copy link
Contributor

@nischal-shetty2 nischal-shetty2 commented Sep 7, 2024

Fixes #833

made the rescan feature available in production, Added translations to the settings page rescan option and for the entire rescan page and also removed the dev tag from this feature as well

@nischal-shetty2
Copy link
Contributor Author

@theborakompanioni could you review this

@theborakompanioni
Copy link
Collaborator

theborakompanioni commented Sep 7, 2024

@theborakompanioni could you review this

Nice!

Regarding the i18n keys: It is tried - although not 100% consistent - to have them prefixed with their intent, e.g. "text_button_submit", "text_button_submitting" or "label_<text>" instead of "submit_button" or "blockheight_label".
Okay for you to adapt that?

Edit: Do you think we need a better/longer description text (subtitle)?

@nischal-shetty2
Copy link
Contributor Author

nischal-shetty2 commented Sep 7, 2024

Regarding the i18n keys: It is tried - although not 100% consistent - to have them prefixed with their intent

i was not aware of this, I have made the changes as you've asked
@theborakompanioni these are the final objects

  "rescan_chain": {
    "title": "Rescan timechain (Experimental)",
    "subtitle": "Rescan the local timechain for wallet related transactions.",
    "label_blockheight": "Rescan height",
    "description_blockheight": "The height of the chain at which the rescan process is started.",
    "error_rescanning_failed": "Error while starting the rescan process. Reason: {{ reason }}",
    "feedback_invalid_blockheight": "Please provide a valid value between {{ min }} and the current blockheight.",
    "text_button_submit": "Rescan timechain",
    "text_button_submitting": "Rescanning timechain..."
  },

and

settings: {
     "rescan_chain": "Rescan chain",
}

ill push the changes now

@nischal-shetty2
Copy link
Contributor Author

Edit: Do you think we need a better/longer description text (subtitle)?

The current set of information is clear but this would definitely help the user. could you give any suggestions as to what should be added @theborakompanioni

@theborakompanioni
Copy link
Collaborator

theborakompanioni commented Sep 7, 2024

Edit: Do you think we need a better/longer description text (subtitle)?

The current set of information is clear but this would definitely help the user. could you give any suggestions as to what should be added @theborakompanioni

You are right, however, it is probably okay to do that in a follow-up PR, yes?
I think the changes here are already quite cool - so let's merge it.
After that we should think about disabling the form when a transaction is in progress, e.g. maker or taker running.
This has not been done properly as of now, as it was only available in dev mode. Also worth to do in a follow-up PR. Do you think so as well?

ping @editwentyone

@theborakompanioni theborakompanioni self-requested a review September 7, 2024 13:16
@theborakompanioni theborakompanioni merged commit 3ec72b1 into joinmarket-webui:devel Sep 7, 2024
1 check passed
@nischal-shetty2
Copy link
Contributor Author

You are right, however, it is probably okay to do that in a follow-up PR, yes? I think the changes here are already quite cool - so let's merge it. After that we should think about disabling the form when a transaction is in progress, e.g. maker or taker running. This has not been done properly as of now, as it was only available in dev mode. Also worth to do in a follow-up PR. Do you think so as well?

Yes i agree, we can consider adding better description later on after the rest of the things are sorted!

@nischal-shetty2 nischal-shetty2 deleted the feat/enable_rescan_chain branch September 7, 2024 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(rescan): enable rescanning chain
2 participants