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

Remove abbreviations #551

Merged
merged 27 commits into from
Jan 10, 2024
Merged

Remove abbreviations #551

merged 27 commits into from
Jan 10, 2024

Conversation

garrettmflynn
Copy link
Member

This PR removes abbreviations introduced on the main branch that don't provide enough clarity for developers without significant JavaScript experience.

There were all that I could find on a first pass. If additional abbreviations are identified or found to not be replaced here, we should make them explicit moving forward.

@garrettmflynn garrettmflynn self-assigned this Jan 3, 2024
@CodyCBakerPhD
Copy link
Collaborator

Backend tests are failing here

@@ -186,8 +186,8 @@ export class InstanceManager extends LitElement {
const input = this.shadowRoot.querySelector("#new-info input");
input.focus();

const mousePress = (e) => {
if (!e.composedPath().includes(newInfoDiv)) {
const mousePress = (mousePressEvent) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What would you think about 'clickEvent' here?

Some implicit bias from neurodata experience led to a first reading as 'mouse (species) lever press', which are a type of common LabeledEvent

Copy link
Member Author

Choose a reason for hiding this comment

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

Switching to pointerEvent since that's also a type of event in JS.

@CodyCBakerPhD
Copy link
Collaborator

Some minor comments scattered across; otherwise looks good. Will give thorough hands on test after those are addressed so ensure nothing is unintentionally broken, otherwise will be good to go

Thanks for taking the time to do this

@CodyCBakerPhD CodyCBakerPhD enabled auto-merge (squash) January 10, 2024 18:00
@CodyCBakerPhD CodyCBakerPhD merged commit 77631c6 into main Jan 10, 2024
10 checks passed
@CodyCBakerPhD CodyCBakerPhD deleted the remove-abbreviations branch January 10, 2024 18:03
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