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

Add Pioneer DDJ SB3 documentation #379

Merged
merged 29 commits into from
Jan 10, 2022
Merged

Conversation

jevilalta
Copy link
Contributor

Hi

Submitting the DDJ SB3 documentation. Pull request for the mapping itself is: mixxxdj/mixxx#3821.

@Holzhaus
Copy link
Member

Holzhaus commented May 2, 2021

Thanks! This already looks promising.

Can you add documentation regarding the audio setup and the performance pad modes, e. g.:

@jevilalta
Copy link
Contributor Author

Thanks! This already looks promising.

Can you add documentation regarding the audio setup and the performance pad modes, e. g.:

* https://manual.mixxx.org/2.3/en/hardware/controllers/roland_dj_505.html#audio-setup

* https://manual.mixxx.org/2.3/en/hardware/controllers/roland_dj_505.html#hot-cue-mode

Yep. I'll add it.

@jevilalta
Copy link
Contributor Author

Hi

I've just pushed the latest version with both audio setup and hotcue information. Let me know if anything else is needed.

@Holzhaus
Copy link
Member

Holzhaus commented May 5, 2021

Thanks. Can you please open the files in Inkscape and save them as "Optimized SVGs" to reduce the file size?

@Swiftb0y Swiftb0y self-assigned this May 5, 2021
Copy link
Member

@Holzhaus Holzhaus left a comment

Choose a reason for hiding this comment

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

Thanks. Some comments regarding the button mapping itself.

source/hardware/controllers/pioneer_ddj_sb3.rst Outdated Show resolved Hide resolved
source/hardware/controllers/pioneer_ddj_sb3.rst Outdated Show resolved Hide resolved
@jevilalta
Copy link
Contributor Author

I've saved the SVG files as optimized with the default settings. Some files reduced a good amount but let me know if I should change any defaults. Not an Inkscape power user...

@jevilalta jevilalta requested a review from Swiftb0y May 15, 2021 21:19
@Swiftb0y Swiftb0y requested a review from Holzhaus May 15, 2021 21:45
@Holzhaus
Copy link
Member

Please merge Dancephy#1

@jevilalta
Copy link
Contributor Author

Hi, I am done with the changes requested. Let me know if I missed anything.

Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

Thanks for the high quality figures. Unfortunately, some are a bit inconsistent in their text color, some have transparent text and others have white text (even mixed in the same figure). This doesn't matter much when viewing on a white background, but it does make some text unreadable when viewing the page with inverted colors.

source/hardware/controllers/pioneer_ddj_sb3.rst Outdated Show resolved Hide resolved
source/hardware/controllers/pioneer_ddj_sb3.rst Outdated Show resolved Hide resolved
source/hardware/controllers/pioneer_ddj_sb3.rst Outdated Show resolved Hide resolved
source/hardware/controllers/pioneer_ddj_sb3.rst Outdated Show resolved Hide resolved
source/hardware/controllers/pioneer_ddj_sb3.rst Outdated Show resolved Hide resolved
@jevilalta
Copy link
Contributor Author

So do we want to go with 8 hotcues instead of 4?

@jevilalta
Copy link
Contributor Author

Thanks for the high quality figures. Unfortunately, some are a bit inconsistent in their text color, some have transparent text and others have white text (even mixed in the same figure). This doesn't matter much when viewing on a white background, but it does make some text unreadable when viewing the page with inverted colors.

How do I invert the colors? I am using a dark theme and it does not change the white background on the manual.

@Swiftb0y
Copy link
Member

I'm using a browser plugin called "Dark Reader". I'm not saying you should make the graphics usable to accommodate for this case, it just made me question the quality of SVGs a bit. So I'd like you to ask to go through those graphics and make sure there aren't any hidden surprises such as duplicated objects laying ontop each other and so forth. Issues like that probably also explain the size (850KB is still very large for essentially just a schematic).

@Be-ing
Copy link
Contributor

Be-ing commented Dec 18, 2021

The pad grid SVGs are broken for me:
image

@jevilalta
Copy link
Contributor Author

The pad grid SVGs are broken for me: image

I uploaded a new version of the pad grid. The issue was not obvious to me but re-exported the image with some cleanup.

@jevilalta
Copy link
Contributor Author

i think that's the last of the requested changes, but lmk if anything else needs to be changed

@Swiftb0y
Copy link
Member

Swiftb0y commented Jan 2, 2022

Please fix the stuff pre-commit complains about too

@jevilalta
Copy link
Contributor Author

I've made the last change and re-ran all the pre-commit checks. They all come out ok:
image

@jevilalta
Copy link
Contributor Author

Seems like broken links in one of the other controller's docs is causing the error now.

@Swiftb0y
Copy link
Member

Swiftb0y commented Jan 9, 2022

Thats fine. Ignore it.

@Swiftb0y
Copy link
Member

Swiftb0y commented Jan 9, 2022

Are you comfortable with rebasing for cleaning up the commit history? If not, thats fine too.

@jevilalta
Copy link
Contributor Author

Are you comfortable with rebasing for cleaning up the commit history? If not, thats fine too.

I can give it a shot. Do you want just one commit with everything?

@Holzhaus
Copy link
Member

Holzhaus commented Jan 9, 2022

Are you comfortable with rebasing for cleaning up the commit history? If not, thats fine too.

If you consider this ready to merge, you can also use "squash-and-merge" directly from the GitHub UI.

@Swiftb0y
Copy link
Member

I can give it a shot. Do you want just one commit with everything?

No I would've preferred something more fine grained, but not too fine. I guess its a matter of taste so there isn't really one way to do it right. I think I'll just squash and merge as Jan suggested.

@Swiftb0y Swiftb0y merged commit a4fad0b into mixxxdj:2.3 Jan 10, 2022
@jevilalta jevilalta deleted the pioneer-ddj-sb3 branch January 10, 2022 19:33
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.

5 participants