Skip to content

Conversation

@oruburos
Copy link
Collaborator

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • [x ] is from a uniquely-named feature branch and has been rebased on top of the latest master. (If I was asked to make more changes, I have made sure to rebase onto master then too)
  • [x ] is descriptively named

It also includes the Spanish translation to complete #1549

Translation is partially resolved ( 4/5) due to conflict with date-fns/distance_in_words_to_now
…ranslation

# Conflicts:
#	translations/locales/en-US/translations.json
#	translations/locales/es-419/translations.json
@oruburos
Copy link
Collaborator Author

@andrewn Can you have a look at this, please?

@andrewn
Copy link
Member

andrewn commented Aug 22, 2020

@oruburos Can you fix the merge conflicts please?

andrewn
andrewn previously approved these changes Aug 22, 2020
Copy link
Member

@andrewn andrewn left a comment

Choose a reason for hiding this comment

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

Looks good

@andrewn
Copy link
Member

andrewn commented Aug 22, 2020

This looks good, just noticed in Editor.jsx:

this._cm.on('keyup', () => {
      const temp = `line ${parseInt((this._cm.getCursor().line) + 1, 10)}`;
      document.getElementById('current-line').innerHTML = temp;
    });

This inserts "line 6" into the DOM for screenreaders to discover, so this should also be translated. If you'd like to do this in another PR, then can you open an issue so we don't forget.

…ranslation

# Conflicts:
#	translations/locales/en-US/translations.json
#	translations/locales/es-419/translations.json
@oruburos
Copy link
Collaborator Author

oruburos commented Aug 23, 2020

@andrewn, I've merged the conflicts and introduced the code to complete the keyUp functionality regarding #1567 (comment) ,
Can you have a look at this, please?

I edited this comment, I pushed again my branch and all the checks have passed

Copy link
Member

@andrewn andrewn left a comment

Choose a reason for hiding this comment

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

👍

@andrewn andrewn merged commit c840734 into processing:develop Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants