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 followed tags to the mobile menu #5468

Merged
merged 2 commits into from Jan 6, 2015

Conversation

Projects
None yet
2 participants
@Flaburgan
Copy link
Member

Flaburgan commented Dec 9, 2014

I had two bad reviews on the mozilla marketplace because the mobile version doesn't allow to access followed tags, so here we go :)

tag = normalized_tag_name
link_to("##{tag}", tag_path(:name => tag))
def tag_link(tag)
link_to("##{tag}", tag_path(name: ActsAsTaggableOn::Tag.normalize(tag)))
end

This comment has been minimized.

@jhass

jhass Dec 9, 2014

Member

Why did you inline this method?

@jhass

This comment has been minimized.

Copy link
Member

jhass commented Dec 9, 2014

How about some tests?

@Flaburgan Flaburgan force-pushed the Flaburgan:add-followed-tags-to-mobile-menu branch 2 times, most recently from 8306d97 to faf97af Dec 9, 2014

@Flaburgan

This comment has been minimized.

Copy link
Member

Flaburgan commented Dec 10, 2014

@jhass what do you want me to test? The whole mobile view is not tested... So we should definitely write some but not related to this PR imo.

@jhass

This comment has been minimized.

Copy link
Member

jhass commented Dec 10, 2014

Adding tests for the existing functionality is, adding tests for the functionality that this adds is not.

At least test that a link for each of the users followed tags is generated.

@Flaburgan Flaburgan force-pushed the Flaburgan:add-followed-tags-to-mobile-menu branch from faf97af to c27041c Dec 28, 2014

@Flaburgan

This comment has been minimized.

Copy link
Member

Flaburgan commented Dec 28, 2014

I added tests to test the whole navigation (header, drawer, search bar) on the mobile version.


Scenario: search a user
When I open the drawer
And I search for "Bob"

This comment has been minimized.

@Flaburgan

Flaburgan Dec 28, 2014

Member

https://github.com/diaspora/diaspora/blob/develop/features/step_definitions/custom_web_steps.rb#L178

Makes the test to fail on mobile. Any idea why this "find()" method is useful?

@Flaburgan Flaburgan force-pushed the Flaburgan:add-followed-tags-to-mobile-menu branch from c27041c to 3ff2986 Dec 28, 2014

@Flaburgan

This comment has been minimized.

Copy link
Member

Flaburgan commented Dec 28, 2014

New (and old) tests are now green :)

@jhass

This comment has been minimized.

Copy link
Member

jhass commented Jan 6, 2015

Thank you!

@jhass jhass merged commit 3ff2986 into diaspora:develop Jan 6, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@jhass jhass added this to the next-major milestone Jan 6, 2015

jhass added a commit that referenced this pull request Jan 6, 2015

Merge pull request #5468 from Flaburgan/add-followed-tags-to-mobile-menu
Add followed tags to the mobile menu

Conflicts:
	Changelog.md
@Flaburgan

This comment has been minimized.

Copy link
Member

Flaburgan commented Jan 6, 2015

:D cool, thank you!

@Flaburgan Flaburgan deleted the Flaburgan:add-followed-tags-to-mobile-menu branch Jan 6, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment